Skip to content

Добавил всё#68

Open
rurm wants to merge 2 commits intojava-online-course:masterfrom
rurm:features_008
Open

Добавил всё#68
rurm wants to merge 2 commits intojava-online-course:masterfrom
rurm:features_008

Conversation

@rurm
Copy link
Copy Markdown

@rurm rurm commented Sep 14, 2021

Все тесты прошли успешно

private String country;

public Author() {
super();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А зачем здесь явно дергать super?

}

public String getName() {
return name;
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.. Да, в данном случае нет локальной переменной в методе с похожим именем, как в методе setName. Но желательно и в таких случаях использовать this., чтобы довести это правило до автоматизма (это как с включением сигнала поворота даже когда ты один на пустынной дороге).

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) 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.getClass() - обращаешься к методу текущего объекта.

if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Author author = (Author) o;
return name.equals(author.name) && Objects.equals(lastName, author.lastName) && birthdate.equals(author.birthdate) && country.equals(author.country);
Copy link
Copy Markdown

@dmitry-chiginev dmitry-chiginev Sep 14, 2021

Choose a reason for hiding this comment

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

Также, необходимо использовать this, а также не писать слишком длинные строки.
Ну и ко всему прочему и name, и birthdate, и country у тебя могут быть null, поэтому лучше их сравнивать также используя Objects.
Можно написать например так:

Objects.equals(this.name, author.name)
&& Objects.equals(this.lastName, author.lastName)
&& Objects.equals(this.birthdate, author.birthdate)
&& Objects.equals(this.country, author.country) ;

@@ -19,5 +19,64 @@
* 6) Переопределить метод toString с выводом всех полей (не забывайте alt+inset)

This comment was marked as resolved.

private String name;

public Book() {
super();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Также явный вызов super() здесь лишний.


@Override
public String toString() {
return "Book{" +
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Советую использовать String.format, чтобы удобнее и красивее оформить этот код. Почитай как его использовать

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. при обращении к полям и метода текущего объекта.

if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Book book = (Book) o;
return numberOfPages == book.numberOfPages && name.equals(book.name);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Смотри коммент к такому же методу в классе Author

super();
}

public SchoolBook(String authorName, String authorLastName, LocalDate publishDate) {

This comment was marked as resolved.


public class SimpleSchoolBookRepository implements BookRepository<SchoolBook> {
private SchoolBook[] schoolBooks = new SchoolBook[0];
private int numOfBooks = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

У тебя же есть массив schoolBooks, у него есть свойство length. Зачем еще отдельное свойство numOfBooks ?


@Override
public int count() {
return this.numOfBooks;
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.schoolBooks.length возвращать, как и в предыдущем репозитории авторов.


public class SimpleAuthorService implements AuthorService
{
AuthorRepository authorRepository;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Модификатор где? Обычно, такие свойства как репозитории не должны быть доступны извне сервиса. Поэтому указывают модификатор private

AuthorRepository authorRepository;

public SimpleAuthorService() {
super();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Я бы здесь не использовал конструктор по умолчанию. Если создать сервис с помощью этого конструктора и попытаться сохранить автора, что произойдет?

public Author findAuthorByBookName(String name) {
SchoolBook[] books = schoolBookBookRepository.findByName(name);
if (books.length > 0)
return authorService.findByFullName(books[0].getAuthorName(), books[0].getAuthorLastName());
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.

3 participants