-
Notifications
You must be signed in to change notification settings - Fork 153
[유민환] 1주차 java-calculator 미션 제출합니다. #19
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
[유민환] 1주차 java-calculator 미션 제출합니다. #19
Conversation
epicblues
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[코드리뷰] 민성 -> 민환
bosuksh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
민환님 과제하느라 고생 많으셨습니다.
확실히 퀄리티가 좋네요!
아까 리뷰시간에 리뷰하지 못한 부분에 대해서 코멘트도 달았습니다.
또한 질문 주신 부분에 답을 하자면
1. TDD 관련
해당 기능을 다른 클래스가 갖게 된다면 그에 해당하는 테스트코드도 그 클래스의 테스트 코드가 가져가야 하는게 당연하다고 생각합니다.
그리고 테스트코드 역시 관리의 대상이기 때문에 많으면 좋긴 하지만 그만큼 관리가 힘들어질 수 있습니다. 그러하기에 Private 메소드 테스트는 직접적으로 하는게 아닌 그 private 메소드를 사용하는 메소드의 테스트 케이스에서 커버를 해줘야 한다고 생각합니다. 그렇기에 private 메소드를 커버하는 테스트코드를 다양한 값을 통해 테스트해서 검증 할 필요가 있다고 생각합니다.
2. 클래스 분리를 명확하게 하지 못한 것 같습니다.
// 외부에서 OperatorType을 찾아 해당 Type으로 계산 실시
public static double calculate (double firstNumber, double secondNumber) {
return expression.apply(firstNumber, secondNumber);
}저는 개인적으로 후자가 맞다고 생각합니다. Operator을 생성하는거와 계산하는걸 분리시킬 필요가 있다고 생각하고 외부에서 생성한 객체로 단순히 계산만 진행하는게 더 좋을 것 같네요
후위 표기식을 계산하는 클래스와 바꾸는 클래스로 나누는 것이 좋다고 생각하네요
그런데 이걸 각각의 클래스로 만들어도 좋고, 아니면 inner class로 변환/ 계산 이렇게 생성해도 될 것 같네요.
3. 입력값 검증
Validation을 담당하는 클래스를 만들어서 하는건 좋은 방법이지만 유틸성으로 담기엔 검증대상이 너무 많기 때문에 각각 객체에 해당하는 Validator정도는 만드는게 좋다고 생각합니다.
어떤 객체가 조건에 만족하지 못하면 객체 생성을 막는것이 당연하기 때문에, 민환님께서 하신
PostfixExpression처럼 검증을 통해 생성을 막는것도 좋은 방식입니다.
src/main/java/calculator/repository/MemoryResultRepository.java
Outdated
Show resolved
Hide resolved
|
안녕하세요 아만드님! 질문
짧지만 2주라는 기간동안 많이 느끼고 배운 것 같습니다! 앞으로 팀이 어떻게 될지 모르지만 감사했습니다~!~! |
📌 과제 설명
👩💻 요구 사항과 구현 내용
Console
Calculator
MemoryResultRepository
Optinal.empty를 반환합니다.PostifixExpression
OperatorType
✅ PR 포인트 & 궁금한 점
1. TDD 관련
2. 클래스 분리를 명확하게 하지 못한 것 같습니다.
OperatorType에서 연산자를 입력받아 해당하는 타입을 반환하도록 하였습니다.
처음 구현할 때 입력받은 식을 후위표기식으로 바꿔주는 기능을 Paser라는 클래스를 통해 해주었습니다.
3. 입력값 검증