-
Notifications
You must be signed in to change notification settings - Fork 3
[Step1] 리뷰 요청드립니다 :) #2
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
base: jinwook
Are you sure you want to change the base?
Changes from all commits
9deb042
4c2fc90
4ea91cd
6d933ba
e0fdcff
a91070b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package study; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
|
|
||
| public class Equation { | ||
|
|
||
| private final String equation; | ||
|
|
||
| private static final String INVALID_CALCULATION_FORMAT = "잘못된 계산식입니다."; | ||
| private static final String DELIMITER = " "; | ||
|
|
||
| public Equation(String equation) { | ||
| this.equation = equation; | ||
| checkEquation(); | ||
| } | ||
|
|
||
| public void checkEquation() { | ||
| String[] split = equation.split(DELIMITER); | ||
|
|
||
| for (int i = 0; i < split.length; ) { | ||
| checkDoubleNumber(split, i); | ||
| i += 2; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 임의의 숫자를 사용하는 매직 넘버는 소스 코드를 읽기 어렵게 만드는데요 ! 숫자 2는 어떤 의미일까요? |
||
| } | ||
|
|
||
| for (int i = 1; i < split.length; ) { | ||
| checkDoubleSymbol(split, i); | ||
| i += 2; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| public List<Integer> getNumbers() { | ||
| List<Integer> numList = new ArrayList<>(); | ||
| String[] inputArr = equation.split(DELIMITER); | ||
| Arrays.stream(inputArr).filter(this::isParesInt).forEach(s -> addNumList(numList, s)); | ||
| return numList; | ||
| } | ||
|
|
||
| public List<String> getSymbolsList() { | ||
| List<String> symbolList = new ArrayList<>(); | ||
| String[] inputArr = equation.split(DELIMITER); | ||
| Arrays.stream(inputArr).filter(s -> !isParesInt(s)) | ||
| .forEach(s -> addSymbolList(symbolList, s)); | ||
| return symbolList; | ||
| } | ||
|
|
||
| private boolean isParesInt(String input) { | ||
| return input.chars().allMatch(Character::isDigit); | ||
| } | ||
|
|
||
| private void addNumList(List<Integer> numList, String input) { | ||
| numList.add(Integer.parseInt(input)); | ||
| } | ||
|
|
||
| private void addSymbolList(List<String> symbolList, String input) { | ||
| checkSymbol(input); | ||
| symbolList.add(input); | ||
| } | ||
|
|
||
| private void checkSymbol(String input) { | ||
| if (!SymbolStatus.checkSymbol(input)) { | ||
| throw new IllegalStateException(INVALID_CALCULATION_FORMAT); | ||
| } | ||
| } | ||
|
|
||
| private void checkDoubleNumber(String[] split, int i) { | ||
| boolean parseInt = isParesInt(split[i]); | ||
| if (!parseInt) { | ||
| throw new IllegalStateException(INVALID_CALCULATION_FORMAT); | ||
| } | ||
| } | ||
|
|
||
| private void checkDoubleSymbol(String[] split, int i) { | ||
| boolean parse = isParesInt(split[i]); | ||
| if (parse) { | ||
| throw new IllegalStateException(INVALID_CALCULATION_FORMAT); | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package study; | ||
|
|
||
| import java.util.Scanner; | ||
|
|
||
| public class InputView { | ||
|
|
||
| private Scanner scanner; | ||
|
|
||
| public InputView(Scanner scanner) { | ||
| this.scanner = scanner; | ||
| } | ||
|
Comment on lines
+9
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. InputView에서 생성자의 파라미터로 Scanner를 설정해주신 이유가 있을까요? 생성자를 통한 의존성 주입을 통해 얻게되는 장점이 없어보이는 것 같아요 🤔 |
||
|
|
||
| public String readEquation() { | ||
| return scanner.nextLine(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package study; | ||
|
|
||
| import java.util.Scanner; | ||
|
|
||
| public class Main { | ||
|
|
||
| public static void main(String[] args) { | ||
| ResultView resultView = new ResultView(); | ||
|
|
||
| resultView.initStart(); | ||
| Scanner scanner = new Scanner(System.in); | ||
| InputView inputView = new InputView(scanner); | ||
|
|
||
| Equation equation = new Equation(inputView.readEquation()); | ||
| SimpleCalculator simpleCalculator = new SimpleCalculator(equation); | ||
|
|
||
| resultView.viewResult(simpleCalculator); | ||
|
|
||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 파일 마지막에 엔터(개행문자)를 넣어주세요 :) 이유는 리뷰를 진행할 때 깃허브에서 경고메시지를 지우고 혹시 모르는 파일 읽기 오류에 대비하기 위함입니다. 좀 더 알고싶으시면 참고 링크를 보셔도 재밌을 것 같네요 :) Intellij 를 사용하실 경우엔Preferences -> Editor -> General -> Ensure line feed at file end on save 를 체크해주시면파일 저장 시 마지막에 개행문자를 자동으로 넣어줍니다! |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||||||||||||
| package study; | ||||||||||||||||
|
|
||||||||||||||||
| public class ResultView { | ||||||||||||||||
|
|
||||||||||||||||
| public void initStart() { | ||||||||||||||||
| System.out.println("계산식을 입력하세요."); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+5
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 계산식을 입력하라고 콘솔에 출력되는 내용은 InputView에 좀 더 적합해보이기도 하네요. InputView와 ResultView를 구분하는 이유는 요청과 결과를 구분하기 위한 부분이기 때문에 ResultView에서만 출력을 해야하는 것은 아닙니다 :) |
||||||||||||||||
|
|
||||||||||||||||
| public void viewResult(SimpleCalculator simpleCalculator) { | ||||||||||||||||
| int result = simpleCalculator.calEquation(); | ||||||||||||||||
| System.out.println("계산 결과 = " + result); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+9
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 계산기 객체를 viewResult()의 인자로 넘기는 것 보다는 결과 값을 넘겨주는 것은 어떨까요? 계산기 객체와 View 객체를 분리하는 이유는 서로의 의존성을 끊어내고 변경의 영향을 최소화하기 위함입니다. 만약 계산기 객체의 calEquation() 메서드의 네이밍이 바뀌게 된다면 ResultView 에도 변경이 생기지 않을까요?
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,52 @@ | ||||||
| package study; | ||||||
|
|
||||||
| public class SimpleCalculator { | ||||||
|
|
||||||
| private final Equation equation; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
현재 상태에서는 하나의 계산기 객체가 하나의 방정식만을 계산할 수 있는데요. 여러 개의 방정식에 대해 계산할 수 있도록 개선해보면 좋을 것 같습니다 :) |
||||||
|
|
||||||
| private static final String NO_DIVIDE_BY_ZERO = "0으로 나눌 수 없습니다."; | ||||||
|
|
||||||
| public SimpleCalculator(Equation equation) { | ||||||
| this.equation = equation; | ||||||
| } | ||||||
|
|
||||||
| public int cal(String symbol, Integer num1, Integer num2) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메서드나 변수명에 약어를 사용하는 것은 지양해주세요. 약어는 별도의 규약으로 정해지지 않으면 소스코드의 가독성을 어렵게 만듭니다 :)
Suggested change
|
||||||
| if (symbol.equals(SymbolStatus.PLUS.toString())) { | ||||||
| return num1 + num2; | ||||||
| } | ||||||
|
Comment on lines
+14
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SymbolStatus enum 클래스를 만들어주셨네요 👍 SymbolStatus를 통해 연산기호의 |
||||||
|
|
||||||
| if (symbol.equals(SymbolStatus.MINUS.toString())) { | ||||||
| return num1 - num2; | ||||||
| } | ||||||
|
|
||||||
| if (symbol.equals(SymbolStatus.MULTIPLY.toString())) { | ||||||
| return num1 * num2; | ||||||
| } | ||||||
|
|
||||||
| if (symbol.equals(SymbolStatus.DIVISION.toString())) { | ||||||
| checkDivideByZero(num2); | ||||||
| return num1 / num2; | ||||||
| } | ||||||
|
|
||||||
| throw new IllegalStateException("잘못 입력하셨습니다.."); | ||||||
| } | ||||||
|
|
||||||
| private void checkDivideByZero(Integer num) { | ||||||
| if (num == 0) { | ||||||
| throw new ArithmeticException(NO_DIVIDE_BY_ZERO); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| public int calEquation() { | ||||||
| int cal = cal( | ||||||
| equation.getSymbolsList().get(0), | ||||||
| equation.getNumbers().get(0), | ||||||
| equation.getNumbers().get(1)); | ||||||
|
|
||||||
| for (int i = 1; i < equation.getSymbolsList().size(); i++) { | ||||||
| cal = cal(equation.getSymbolsList().get(i), cal, equation.getNumbers().get(i + 1)); | ||||||
| } | ||||||
|
|
||||||
| return cal; | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package study; | ||
|
|
||
| import java.util.Arrays; | ||
|
|
||
| public enum SymbolStatus { | ||
| PLUS("+"), MINUS("-"), MULTIPLY("*"), DIVISION("/"); | ||
|
|
||
| private String value; | ||
|
|
||
| SymbolStatus(String value) { | ||
| this.value = value; | ||
| } | ||
|
|
||
| public String toString() { | ||
| return this.value; | ||
| } | ||
|
|
||
| public static boolean checkSymbol(String inputArr) { | ||
| return Arrays.stream(values()).anyMatch(value -> value.toString().equals(inputArr)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package study; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.util.List; | ||
| import org.junit.jupiter.api.DisplayName; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
|
|
||
| public class EquationTest { | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = {"/", "+"}) | ||
| @DisplayName("연산 기호만 반환") | ||
| void returnSymbolList(String symbol) { | ||
| //given | ||
| String input = "3 / 4 + 5"; | ||
| Equation equation = new Equation(input); | ||
|
|
||
| //when | ||
| List<String> symbols = equation.getSymbolsList(); | ||
|
|
||
| //then | ||
| assertThat(symbols.contains(symbol)).isTrue(); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(ints = {3, 4}) | ||
| @DisplayName("숫자들만 반환") | ||
| void returnIntList(int num) { | ||
| //given | ||
| String input = "3 + 4"; | ||
| Equation equation = new Equation(input); | ||
|
|
||
| //when | ||
| List<Integer> ints = equation.getNumbers(); | ||
|
|
||
| //then | ||
| assertThat(ints.contains(num)).isTrue(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| package study; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class SimpleCalculatorTest { | ||
|
|
||
| @Test | ||
| void plusTest() { | ||
|
Comment on lines
+9
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 계산기 테스트에도 @DisplayName 어노테이션을 활용하면 더 가독성 있는 테스트 코드를 작성 할 수 있을 것 같습니다 :) |
||
| //given | ||
| String input = "10 + 5"; | ||
| Equation equation = new Equation(input); | ||
| SimpleCalculator calculator = new SimpleCalculator(equation); | ||
|
|
||
| //when | ||
| int result = calculator.cal( | ||
| equation.getSymbolsList().get(0), | ||
| equation.getNumbers().get(0), | ||
| equation.getNumbers().get(1)); | ||
|
|
||
| //then | ||
| assertThat(result).isEqualTo(15); | ||
| } | ||
|
|
||
| @Test | ||
| void minusTest() { | ||
| //given | ||
| String input = "10 - 5"; | ||
| Equation equation = new Equation(input); | ||
| SimpleCalculator calculator = new SimpleCalculator(equation); | ||
|
|
||
| //when | ||
| int result = calculator.cal( | ||
| equation.getSymbolsList().get(0), | ||
| equation.getNumbers().get(0), | ||
| equation.getNumbers().get(1)); | ||
|
|
||
| //then | ||
| assertThat(result).isEqualTo(5); | ||
| } | ||
|
|
||
| @Test | ||
| void multiplyTest() { | ||
| //given | ||
| String input = "10 * 5"; | ||
| Equation equation = new Equation(input); | ||
| SimpleCalculator calculator = new SimpleCalculator(equation); | ||
| //when | ||
| int result = calculator.cal( | ||
| equation.getSymbolsList().get(0), | ||
| equation.getNumbers().get(0), | ||
| equation.getNumbers().get(1)); | ||
|
|
||
| //then | ||
| assertThat(result).isEqualTo(50); | ||
| } | ||
|
|
||
| @Test | ||
| void divisionTest() { | ||
| //given | ||
| String input = "10 / 5"; | ||
| Equation equation = new Equation(input); | ||
| SimpleCalculator calculator = new SimpleCalculator(equation); | ||
| //when | ||
| int result = calculator.cal( | ||
| equation.getSymbolsList().get(0), | ||
| equation.getNumbers().get(0), | ||
| equation.getNumbers().get(1)); | ||
|
|
||
| //then | ||
| assertThat(result).isEqualTo(2); | ||
| } | ||
|
|
||
| } | ||
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.
클래스의 구현 컨벤션에 맞춰서 구현하게되면 일관된 코드를 작성할 수 있습니다 :)