[디디] 온보딩 리뷰요청 #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
young891221
suggested changes
Feb 7, 2020
src/main/java/Calculator.java
Outdated
| private double result = 0; | ||
|
|
||
| void calculate() { | ||
| int length = InputValues.getValuesLength(); |
There was a problem hiding this comment.
InputValues를 파라미터로 받도록 변경해 보면 어떨까요?
그렇게 했을 때 이점이 무엇일지 생각해 보세요:)
src/main/java/InputValues.java
Outdated
|
|
||
| public class InputValues { | ||
| private static String value; | ||
| private static String[] values; |
There was a problem hiding this comment.
필드에 데이터값을 담게 되면 멀티 쓰레드 환경 처리에서 문제가 있습니다.
이보다는 변수로 담아서 전달하도록 변경해 주세요.
src/main/java/InputValues.java
Outdated
| value = scanner.nextLine(); | ||
| values = value.split(" "); | ||
| validateDouble(); | ||
| validateOperator(); |
There was a problem hiding this comment.
한번만 돌아도 될 for문을 여러번 돌리고 있다는 생각이 듭니다. calculate()까지 합치면 3번을 돌리게 되는데 모두 한번에 처리할 수 있지 않을까 생각되네요:)
InputValues 의 메소드들과 변수를 static 에서 인스턴스 필드와 메소드로 변경 OutputView 작성을 통해 결과 출력 클래스를 만듬 Operator Enum 의 operate 메소드를 람다식의 BinaryOperator 를 이용해서 재작성 InputValues 의 Validation 방식 변경 (숫자와 연산자를 함께 검증하도록 만듬) Calculator Test 의 테스트 방식 변경(올바른 입력값이 들어온 경우와 아닌 경우를 나눠서 검증) CalculatorTest가 예상된 Exception 을 정확하게 catch 하도록 수정
young891221
approved these changes
Feb 9, 2020
| } | ||
|
|
||
| return operate2((s, t) -> s / t, num1, num2); | ||
| } |
Input 클래스 추가 - InputValue 의 필드를 포장 Input 클래스 추가로 인한 나머지 클래스 리팩토링
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
감사합니다.