Skip to content
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

Task8 by Lesiv K #11

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

Task8 by Lesiv K #11

wants to merge 2 commits into from

Conversation

mak-42
Copy link
Owner

@mak-42 mak-42 commented Nov 3, 2023

No description provided.

*/
@Test
public void parseSomethingInside() {
public void parseSomethingInside() throws IllegalPositionException {
Copy link
Owner Author

Choose a reason for hiding this comment

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

При выдаче задания предупреждали, что текст тестов должен оставаться неизменным. Если вам понадобилось его изменить, возможно, вы что-то реализовали неправильно.

public void check(String[] positions) throws IllegalMoveException {
ChessPosition pos;
try {
pos = ChessPositionParser.parse(positions[0]);
Copy link
Owner Author

Choose a reason for hiding this comment

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

Мне нравится ваш подход, но надо немного его доработать. Здесь получим непонятное для пользователя исключение, если он по ошибке не указал ни одной клетки в качестве аргумента запуска программы.

}
pos = nextPos;
}
System.out.print("OK");
Copy link
Owner Author

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.

2 participants