Skip to content

Conversation

@Binary-Cat-01
Copy link

No description provided.

settings.gradle Outdated
@@ -1,2 +1,3 @@
rootProject.name = 'unit-testing-practical-task'


Copy link
Owner

Choose a reason for hiding this comment

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

лишняя пустая строка


FullName fullName = (FullName) o;
return Objects.equals(name, fullName.name) && Objects.equals(surname, fullName.surname) && Objects.equals(
patronymic, fullName.patronymic);
Copy link
Owner

Choose a reason for hiding this comment

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

Лучше сразу часть выражения перенести на следующую строку:

&& Objects.equals(patronymic, fullName.patronymic)

Всегда усложняет восприятие, если переносишь что-то вложенное, как здесь

fullNameValidationService.validatePatronymic(patronymic);

return new FullName(name, surname, patronymic);
} catch (RegexValidationException e) {
Copy link
Owner

Choose a reason for hiding this comment

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

В чем смысл try-catch? Ты ведь буквально подменяешь один RuntimeException другим

В целом, явное отлавливание RuntimeException в catch будет некорректным подходом в абсолютном большинстве случаев. Есть ситуации, когда это оправдано, но когда ты с ними столкнешься - скорее всего уже придет понимание, зачем это делать

public static final String NAME_REGEX = "[А-Я][а-я]*";
public static final String PATRONYMIC_REGEX = "[А-Я][а-я]+";

public FullNameValidationService() {
Copy link
Owner

Choose a reason for hiding this comment

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

конструктор по умолчанию будет создан и без твоего участия

public static final String FULL_NAME_REGEX = "^[А-Я][А-Яа-я-]* [А-Я][а-я]* [А-Я][а-я]+$";
public static final String DOUBLE_SURNAME_REGEX = "[А-Я][а-я]*-[А-Я][а-я]*";
public static final String NAME_REGEX = "[А-Я][а-я]*";
public static final String PATRONYMIC_REGEX = "[А-Я][а-я]+";
Copy link
Owner

Choose a reason for hiding this comment

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

Если константа применяется только внутри класса - почему public?

import static org.junit.jupiter.api.Assertions.*;

class FullNameValidationServiceTest {
private FullNameValidationService fullNameValidationService;
Copy link
Owner

Choose a reason for hiding this comment

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

почему поле расположено выше констант?

}

@ParameterizedTest
@EmptySource
Copy link
Owner

Choose a reason for hiding this comment

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

зачем?

Copy link
Owner

Choose a reason for hiding this comment

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

И выше где-то аналогичная аннотация

@ParameterizedTest
@EmptySource
@FieldSource("INVALID_DOUBLE_SURNAMES")
void validateSurname_invalidSurNames_ThrowsException(String invalidSurName) {
Copy link
Owner

Choose a reason for hiding this comment

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

validateSurname_invalidSurNames - кажется, в большинстве методов вторую часть можно сделать лаконичнее. Ну и surname - это одно слово)

// then
assertDoesNotThrow(actual);
}
} No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

не увидел теста под ситуации, когда в метод(-ы) передается null


@ParameterizedTest
@FieldSource("VALID_PATRONYMICS")
void validatePatronymic_validPatronymics_DoesNotThrowException(String validPatronymic) {
Copy link
Owner

Choose a reason for hiding this comment

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

DoesNotThrowException - как будто переусложнено. Отсутствие исключение - целевое поведение для абсолютного большинства методов (по сути, всех, которые не бросают UnsupportedOperation). Зачем это дополнительно подчеркивать?

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