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

문자열 계산기 구현 #4

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

Conversation

jjanggyu
Copy link

No description provided.

@begaonnuri begaonnuri self-requested a review February 26, 2021 01:54
songpang pushed a commit to songpang/java-calculator that referenced this pull request Feb 26, 2021
@jjanggyu
Copy link
Author

문자열 계산기 코드 일치 완료!

@songpang
Copy link
Member

코드에 대해서 딱히 문제점을 찾지 못했어요. 코드 잘짜셨네요, 고생하셨습니다!!

Copy link
Member

@begaonnuri begaonnuri left a comment

Choose a reason for hiding this comment

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

잘 구현해주셨네요 👍🏻
몇가지 코멘트 남겼으니 확인해주세요!

@@ -2,7 +2,7 @@ apply plugin: 'java'
apply plugin: 'eclipse'

version = '1.0.0'
sourceCompatibility = 1.8
sourceCompatibility = 11
Copy link
Member

Choose a reason for hiding this comment

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

오 11버전을 사용하셨군요! 이유가 있을까요?

Comment on lines +15 to +17
private Printer printer;
private Receiver receiver;
private Calculator calculator;
Copy link
Member

Choose a reason for hiding this comment

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

한번 할당 후 변경이 없는 값들은 final을 붙여주는것이 좋습니다!
final에 대해 학습해보세요

Comment on lines +4 to +11
private final String GREET_MESSAGE = "계산기 프로그램입니다.\n" +
"숫자와 사칙연산 기호를 공백으로 구분하여 입력해주세요.\n" +
"프로그램을 종료하시려면 exit을 입력해주세요.";

private final String INPUT_REQUEST_MESSAGE = "식 입력: ";

private final String OUTPUT_MESSAGE = "answer: %d\n";
private final String EXIT_MESSAGE = "프로그램을 종료합니다.";
Copy link
Member

Choose a reason for hiding this comment

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

상수처리 👍🏻

Comment on lines +3 to +12
public interface Printer {

void greet();

void printWaitingForInputText();

void printResult(int result);

void printExitMessage();
}
Copy link
Member

Choose a reason for hiding this comment

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

인터페이스 👍🏻
하지만 개인적으로 이렇게 하나의 클래스만 사용되는 경우 인터페이스 추출은 오버엔지니어링이라고 생각되긴 합니다!


public class ConsoleReceiver implements Receiver {

private final Scanner scanner;
Copy link
Member

Choose a reason for hiding this comment

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

애플리케이션이 여러번 실행될때 매번 Scanner가 생성되고 있어요.
static을 활용해서 한 번만 생성하도록 하는건 어떨까요?
static에 대해 학습해보세요.

Comment on lines +6 to +13
if (operator.equals("+")) {
return new Addition();
} else if (operator.equals("-")) {
return new Subtraction();
} else if (operator.equals("*")) {
return new Multiplication();
}
return new Division();
Copy link
Member

Choose a reason for hiding this comment

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

지금은 연산자가 4개 뿐이지만 연산자와 비례해서 이 메소드의 길이가 증가할것같아요.
if문의 분기를 어떻게 없앨 수 있을까요?
상속이 아닌 인터페이스 구현을 통해 다형성을 활용해보세요.

Comment on lines +15 to +22
int test1Result = calculator.calculate("1 + 1");
assertThat(test1Result).isEqualTo(2);

int test2Result = calculator.calculate("3 + 7");
assertThat(test2Result).isEqualTo(10);

int test3Result = calculator.calculate("10 + 234");
assertThat(test3Result).isEqualTo(244);
Copy link
Member

Choose a reason for hiding this comment

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

테스트 작성 👍🏻
여기서 1+1, 3+7, 10+234를 나누셨는데 어떤 기준으로 나누셨나요?
어떻게 해야 의미있는 테스트를 작성할 수 있을까요?

void 나누기는_0으로_나눌때_DivideByZeroException_발생() {
assertThatExceptionOfType(DivideByZeroException.class)
.isThrownBy(() -> calculator.calculate("2 / 0"))
.as("0으로 나눌 수 없습니다.");
Copy link
Member

Choose a reason for hiding this comment

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

as메소드의 docs엔 어떤 말이 적혀있나요?
assertThatExcpetionOfType 메소드의 docs엔 어떤 말이 적혀있고, 어떻게 사용되고있나요?

}

@Test
void 정상적이지_않은_공백_입력에_대해서도_올바른_답을_낼_수_있다() {
Copy link
Member

Choose a reason for hiding this comment

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

상세한 예외케이스 👍🏻

assertThat(test2Result).isEqualTo(7);
}

@Test
Copy link
Member

Choose a reason for hiding this comment

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

JUnit5의 ParameterizedTest를 활용해보세요!
인자를 통해 메소드의 길이를 단축시킬 수 있습니다!

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