Skip to content

Update all classes#1

Open
maryvictol wants to merge 2 commits intomasterfrom
workbranch
Open

Update all classes#1
maryvictol wants to merge 2 commits intomasterfrom
workbranch

Conversation

@maryvictol
Copy link
Copy Markdown
Owner

No description provided.

public boolean equals(Object compareObject) {
if (compareObject == null) {
return false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно ещё сразу проверить на совпадение ссылок: this == compareObject

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

добавила

}

@Override
public boolean equals(Object compareObject) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если Вы переопределяете в классе метод equals, то нужно переопределять и hashcode обязательно

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

переопределила

if (actor == null) {
throw new IllegalArgumentException("Actors List shouldn't be null.");
}
this.mainFilmActors = new HashSet<>();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а если какие-то актёры уже были добавлены, мы же не хотим, чтобы один актёр затёр всех остальных? Я бы предложила проверять, если this.mainFilmActors == null, то тогда создавать новый, а иначе просто добавлять к уже существующему

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

перенесла this.mainFilmActors = new HashSet<>(); в конструктор, а в методе setMainActors оставила добавление в коллекцию

throw new IllegalArgumentException("Actors List shouldn't be null.");
}
this.mainFilmActors = new HashSet<>();
this.mainFilmActors = mainFilmActors;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

аналогично предыдущему, проверила бы на nullи потом добавляла бы к новому или уже существующему через addAll

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

аналогично предыдущему

}

@Override
public boolean equals(Object compareObject) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

надо добавить реализацию для hashCode

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

добавила

public boolean equals(Object compareObject) {
if (compareObject == null) {
return false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

сравнить ссылки

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

добавила

if (films == null) {
throw new IllegalArgumentException("Films list shouldn't be null.");
}
this.films=films;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше через addAll, иначе будут ссылаться на одно место в памяти

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исправила




public void deserialisationCollection(String fileName) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а почему не возвращаете FilmsCollection? из-за него же всё и затевается

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исправила

import static org.junit.Assert.*;

public class FilmsCollectionTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

здесь было бы как раз полезно потестировать десериализацию

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants