Oereo : Java-calculator #5
Conversation
Merge feature/java-calculator
refactor: delete unused class and fix naming
feat: add TDD basic format and test case for calculator class
feat: add comment and refactor
feat: add invalid test case
There was a problem hiding this comment.
다른 분들은 어떤 식으로 구현했는지 분석하고 리뷰 달면서 스스로도 공부할 수 있어서 좋았습니다 ㅎㅎ 메서드명과 변수명만 수정해주면 더 좋은 코드가 될 것 같습니다! 수고하셨습니다 :)
자바 변수 네이밍, 자바 메서드 네이밍
| * | ||
| * return type : void | ||
| */ | ||
| public int NumericalError(String number) { |
There was a problem hiding this comment.
메서드명은 소문자 카멜표기법을 사용하고, 동사나 전치사로 시작하기 때문에 checkNumericalException 이런 식으로 메서드에 맞게 바꾸는게 나을 것 같아요!
|
|
||
| import java.util.InputMismatchException; | ||
|
|
||
| public class ErrorException { |
There was a problem hiding this comment.
이 클래스는 NumberFormatException 에 관한 메서드밖에 없으니 클래스명을 그에 맞게 바꾸는건 어떨까요?
|
|
||
|
|
||
|
|
||
|
|
| package calculator; | ||
|
|
||
| interface OperationInterface { | ||
| public int operationPriority(); |
There was a problem hiding this comment.
이 함수는 사용되지 않고 OperationInterface를 어떤 용도로 구현하려한건지 궁급합니다!
그리고 이 인터페이스를 상속받는 모든 클래스에서 다 return 1;로 같은 값을 반환하던데 추상 클래스로 구현해도 될 것 같다는 생각이 드네요
|
|
||
| @Override | ||
| public int calculation(int beforeNumber, int afterNumber) { | ||
| int Result = beforeNumber - afterNumber; |
There was a problem hiding this comment.
변수는 첫 글자가 소문자로 시작해야하므로 result로 바꾸면 좋을 것 같아요~
|
|
||
| // Raise error when a symbol that does not correspond to the arithmetic operations(+, -, *, /) comes | ||
| else{ | ||
| message.exceptionResult("NOT_OPERATOR"); |
There was a problem hiding this comment.
Message.exceptionResult()는 예외를 던지지 않고 메세지만 출력해주는 메서드인것 같아요
이 경우에는 for문이 계속 돌지 않을까요?
| if(cumulativeResult != 0){ | ||
| message.calculationResult(cumulativeResult); | ||
| } |
There was a problem hiding this comment.
2 - 2같이 결과값이 0이 나오는 경우도 고려해주어야 할 것 같아요
|
|
||
| @Test | ||
| void addChooseOperatorAndCalculate(){ | ||
| Calculator calculator = new Calculator(); |
| } | ||
| }); | ||
| } | ||
| } |
| @Test | ||
| void calculation() { | ||
| } |
| public class Application { | ||
| public static void main(String[] args) { | ||
|
|
||
| // Declare calculator object and start calculation |
There was a problem hiding this comment.
주석은 코드로 표현할 수 없는 상황에만 적어주세요.
최대한 주석을 사용하지 않고 코드를 이해할 수 있게 만드는게 좋습니다 🙂
| interface Stack{ | ||
| boolean isEmpty(); | ||
| String pop(); | ||
| void clear(); | ||
| } |
There was a problem hiding this comment.
java.util.Stack이 이미 존재한답니다. 따로 구현해줄 이유가 있었을까요?
java.util의 Stack을 활용하여 ArithmeticExpressionStack 클래스를 리팩토링 해보도록해요 🙂
바퀴의 재발명
이미 있는 라이브러리나 프레임워크를 다시 만들지 말라.
| /** | ||
| * calculation method | ||
| * 1. getEquation - Receives numeric expression in string format entered by user | ||
| * 2. Put numeric expressions on the stack | ||
| * 3. Passing the stack to the operatorSetting method | ||
| * | ||
| * return type : void | ||
| */ |
There was a problem hiding this comment.
javadoc을 만들땐 @를 활용하여 tag를 사용할 수 있습니다. (@param, @exception 등)
하지만 라이브러리를 만드는것이 아닌 어플리케이션을 만드는 과정이니, 주석이나 javadoc이 필수는 아닙니다.
최대한 코드만 읽어도 이해할 수 있도록 만드는걸 목표로 해보세요.
(클린코드의 저자 martin은 명쾌한 추상화라는 표현을 굉장히 좋아했었습니다.)
|
|
||
|
|
||
| public class Calculator { | ||
| Message message = new Message(); |
There was a problem hiding this comment.
멤버 변수의 접근 제어자가 default인데 의도한 부분인가요?
| } | ||
|
|
||
| // When the operator is equal to "-" | ||
| else if (operator.equals(subOperation.operationName())){ |
There was a problem hiding this comment.
if ~ else 체인을 최대한 사용하지 말아보세요. 다형성이나 enum을 활용하면 해결할 수 있습니다.
| @Test | ||
| void invalidCalculation(){ | ||
| // todo refactoring | ||
| assertThrows(NumberFormatException.class, new Executable() { |
| @BeforeEach | ||
| void setUp() { | ||
| Calculator calculator = new Calculator(); | ||
| } | ||
|
|
||
| @Test | ||
| void calculation() { | ||
| } | ||
|
|
||
| @Test | ||
| void operatorSetting() { | ||
| } |
There was a problem hiding this comment.
사용되지 않는 부분들이네요!
| @BeforeEach | |
| void setUp() { | |
| Calculator calculator = new Calculator(); | |
| } | |
| @Test | |
| void calculation() { | |
| } | |
| @Test | |
| void operatorSetting() { | |
| } |
| @@ -0,0 +1,25 @@ | |||
| package calculator; | |||
|
|
|||
| import java.util.InputMismatchException; | |||
| * return type : void | ||
| */ | ||
| public class Message { | ||
| HashMap<String, String> exceptionMessageList = new HashMap<String, String>(); |
There was a problem hiding this comment.
에러처리는 Exception을 활용하여 처리할 수 있습니다.
System.exit 대신 자바의 Exception을 활용해보세요 🙂
|
|
||
| @Override | ||
| public int calculation(int beforeNumber, int afterNumber) { | ||
| int Result = beforeNumber * afterNumber; |
There was a problem hiding this comment.
클래스의 시작은 대문자, 변수의 시작은 소문자입니다 🙂
| int Result = beforeNumber * afterNumber; | |
| int result = beforeNumber * afterNumber; |
java-calculator