Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

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

Open
rurm wants to merge 2 commits intojava-online-course:masterjava-online-course/java-oop-template:masterfrom
rurm:features_008rurm/java-oop-template:features_008Copy head branch name to clipboard
Open

Добавил всё#68
rurm wants to merge 2 commits intojava-online-course:masterjava-online-course/java-oop-template:masterfrom
rurm:features_008rurm/java-oop-template:features_008Copy head branch name to clipboard

Conversation

@rurm
Copy link

@rurm rurm commented Sep 14, 2021

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

private String country;

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.

private String name;

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.