Skip to content

Feature/hw 05#1

Open
uoles wants to merge 16 commits intomasterfrom
feature/hw-05
Open

Feature/hw 05#1
uoles wants to merge 16 commits intomasterfrom
feature/hw-05

Conversation

@uoles
Copy link
Copy Markdown
Owner

@uoles uoles commented May 30, 2019

No description provided.

@Data
public class Book {

private int id;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Для идентификатора лучше использовать long + аналог для схемы БД

private Date addRecordDate;
private String caption;
private int authorId;
private int genreId;
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 и Genre

this.comment = comment;
}

public String getAddRecordDateString() {
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 String getAuthorString() {
return (author != null)
? ", author='" + author.toFormatedString() + '\''
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Я так понимаю все эти методы нужны, чтобы выводить нужным образом в интерфейсе. Лучше не подгонять код сущностей под нужды интерфейса. Потенциально они могут быть использованы в разных приложениях. Для подготовки данных к отображению лучше использовать сервис или dto

@RequiredArgsConstructor
public class AuthorDaoJdbc implements AuthorDao<Author> {

private final JdbcOperations jdbcOperations;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Лучше использовать NamedParameterJdbcOperations

ADD_RECORD_DATE DATE,
CAPTION VARCHAR(255) not null,
AUTHOR_ID INT not null,
GENRE_ID INT not null,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Если удалить автора или жанра с книгами то они удалятся, а книги останутся с идентификаторами для которых нет записей в соответствующих таблицах. Лучше обеспечить целостность данных на уровне схем БД с помощью внешних ключей

@Override
public Book getFullBookById(int id) {
Book book = bookDao.getById(id);
return setBookInfo(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.

Такой подход приведет к n*2 + 1 запросам к БД (все книги + (жанры + авторы) * количество книг). Т.о. запрос 1000 книг выльется в 2001 запрос. При текущей структуре Book лучше во время запроса книг связать таблицы книг, авторов и жанров и получить все за один раз.


@DisplayName("Класс AuthorDaoJdbc")
@RunWith(SpringRunner.class)
@SpringBootTest(classes = AppTestConfig.class)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Дао Jdbc лучше тестировать через @JdbcTest + @Import нужного дао (или @ComponentScan )

.addScript("test_data.sql")
.build();

authorDaoJdbc = new AuthorDaoJdbc(new JdbcTemplate(db));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. Настройку DataSource лучше сделать в application.yml (spring.datasource, spring.datasource.data). H2 буде по умолчанию если она есть pom (scope test)
  2. Схема у приложения и тестов лучше, чтобы была одна. Т.е. "test_schema.sql" не нужно указывать
  3. Репозитории в тесты с контекстом лучше внедрить (@Autowired).

Maksim Kulikov added 2 commits May 31, 2019 16:30
- Изменены запросы книг.
- Добавление внешних ключей для таблицы книг.
- Изменение сущности Book.
- Изменение типа id.
- Использование @jdbcTest.
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