Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

rurm
Copy link

@rurm rurm commented Sep 14, 2021

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


public Author() {
super();

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;

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;

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

@DmitryChiginev DmitryChiginev 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.


public Book() {
super();

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{" +

Choose a reason for hiding this comment

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

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

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);

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;

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;

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;

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();

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());

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