[Step1] 리뷰 요청드립니다 :) #2
Conversation
- [x] 문자열을 문자들로 분리
- [x] 분리된 문자들 중 숫자만 반환
- [x] 분리된 문자들 중 연산기호만 반환
- [x] 주어진 사칙연산기호를 통해 두 개의 숫자를 계산한다.
- 사칙연산 기호(symbol)
- [x] ->두 개의 숫자를 덧셈
- [x] ~/java/TDD/java-string-calculator->두 개의 숫자를 뺄셈
- [x] ->두 개의 숫자를 곱셈
- [x] ->두 개의 숫자를 나눗셈
- [x] 0으로 나누면 throw new ArithmeticException();
| private final String equation; | ||
|
|
||
| private static final String INVALID_CALCULATION_FORMAT = "잘못된 계산식입니다."; | ||
| private static final String DELIMITER = " "; |
There was a problem hiding this comment.
클래스의 구현 컨벤션에 맞춰서 구현하게되면 일관된 코드를 작성할 수 있습니다 :)
class A {
상수(static final) 또는 클래스 변수
인스턴스 변수
생성자
팩토리 메소드
메소드
기본 메소드 (equals, hashCode, toString)
}
| public InputView(Scanner scanner) { | ||
| this.scanner = scanner; | ||
| } |
There was a problem hiding this comment.
InputView에서 생성자의 파라미터로 Scanner를 설정해주신 이유가 있을까요?
생성자를 통한 의존성 주입을 통해 얻게되는 장점이 없어보이는 것 같아요 🤔
| public void initStart() { | ||
| System.out.println("계산식을 입력하세요."); | ||
| } |
There was a problem hiding this comment.
계산식을 입력하라고 콘솔에 출력되는 내용은 InputView에 좀 더 적합해보이기도 하네요.
InputView와 ResultView를 구분하는 이유는 요청과 결과를 구분하기 위한 부분이기 때문에 ResultView에서만 출력을 해야하는 것은 아닙니다 :)
| public void viewResult(SimpleCalculator simpleCalculator) { | ||
| int result = simpleCalculator.calEquation(); | ||
| System.out.println("계산 결과 = " + result); | ||
| } |
There was a problem hiding this comment.
계산기 객체를 viewResult()의 인자로 넘기는 것 보다는 결과 값을 넘겨주는 것은 어떨까요?
계산기 객체와 View 객체를 분리하는 이유는 서로의 의존성을 끊어내고 변경의 영향을 최소화하기 위함입니다.
만약 계산기 객체의 calEquation() 메서드의 네이밍이 바뀌게 된다면 ResultView 에도 변경이 생기지 않을까요?
| public void viewResult(SimpleCalculator simpleCalculator) { | |
| int result = simpleCalculator.calEquation(); | |
| System.out.println("계산 결과 = " + result); | |
| } | |
| public void viewResult(int result) { | |
| System.out.println("계산 결과 = " + result); | |
| } |
| @Test | ||
| void plusTest() { |
There was a problem hiding this comment.
계산기 테스트에도 @DisplayName 어노테이션을 활용하면 더 가독성 있는 테스트 코드를 작성 할 수 있을 것 같습니다 :)
|
|
||
| for (int i = 0; i < split.length; ) { | ||
| checkDoubleNumber(split, i); | ||
| i += 2; |
There was a problem hiding this comment.
임의의 숫자를 사용하는 매직 넘버는 소스 코드를 읽기 어렵게 만드는데요 !
숫자 2는 어떤 의미일까요?
| resultView.viewResult(simpleCalculator); | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
파일 마지막에 엔터(개행문자)를 넣어주세요 :)
이유는 리뷰를 진행할 때 깃허브에서 경고메시지를 지우고 혹시 모르는 파일 읽기 오류에 대비하기 위함입니다.
좀 더 알고싶으시면 참고 링크를 보셔도 재밌을 것 같네요 :)
Intellij 를 사용하실 경우엔Preferences -> Editor -> General -> Ensure line feed at file end on save 를 체크해주시면파일 저장 시 마지막에 개행문자를 자동으로 넣어줍니다!
|
|
||
| public class SimpleCalculator { | ||
|
|
||
| private final Equation equation; |
There was a problem hiding this comment.
계산기가 하나의 방정식을 상태로 가지도록 구현하셨네요 !
계산기가 또 다른 방정식에 대한 값은 어떻게 구할 수 있을까요?
현재 상태에서는 하나의 계산기 객체가 하나의 방정식만을 계산할 수 있는데요. 여러 개의 방정식에 대해 계산할 수 있도록 개선해보면 좋을 것 같습니다 :)
| this.equation = equation; | ||
| } | ||
|
|
||
| public int cal(String symbol, Integer num1, Integer num2) { |
There was a problem hiding this comment.
메서드나 변수명에 약어를 사용하는 것은 지양해주세요.
약어는 별도의 규약으로 정해지지 않으면 소스코드의 가독성을 어렵게 만듭니다 :)
| public int cal(String symbol, Integer num1, Integer num2) { | |
| public int calculate(String symbol, Integer num1, Integer num2) { |
| if (symbol.equals(SymbolStatus.PLUS.toString())) { | ||
| return num1 + num2; | ||
| } |
There was a problem hiding this comment.
SymbolStatus enum 클래스를 만들어주셨네요 👍
SymbolStatus를 통해 연산기호의 상태를 한 곳에서 관리하게 해주셨으니 계산을 하는 행위도 함께 enum에 구현해보면 어떨까요?
2 participants
어렵다 ㅠㅠ