Skip to content
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

[라빈] 온보딩 - 학습 테스트, 단위 테스트 #3

Merged
merged 18 commits into from
Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ed7ba12
add : 요구사항 1 - split 했을 때 contains()를 활용해 변환 값이 맞는지 검증하는 테스트 코드 작성
giantim Feb 5, 2020
b7f56bf
add : 요구사항 2 - substring()을 활용해 올바른 문자열이 반환되는지 테스트
giantim Feb 5, 2020
4646843
add : 요구사항 3 - charAt() 메소드를 활용해 범위를 넘어가는 예외 테스트
giantim Feb 5, 2020
3994dee
add : 요구사항 1 - size() 메소드를 활용해 Set의 크기를 확인하는 테스트
giantim Feb 5, 2020
db637c0
add : 요구사항 2 - JUnit의 ParameterizedTest를 이용해 중복 코드를 제거
giantim Feb 5, 2020
1da46a8
add : 요구사항 3 - @CsvSource를 활용해 입력 값에 따라 결과 값이 다른 경우의 테스트
giantim Feb 5, 2020
43d67f6
add : 문자열 계산기 기능 구현 및 테스트 코드 작성
giantim Feb 7, 2020
75191ca
add : README 수정
giantim Feb 7, 2020
f5b3b85
Update README.md
giantim Feb 7, 2020
19caade
refact : operator를 enum 클래스로 만들어서 관리 / Validator -> ValidityInspector…
giantim Feb 8, 2020
006eded
refact : if 문의 나열 -> if ~ else if 문으로 변경 / 조건문의 단계별로 return
giantim Feb 8, 2020
470960a
refact : 입력 문자열의 공백 또는 null 검사 메소드를 하나로 통일 / 매직넘버 제거 / checkIsDoubleN…
giantim Feb 8, 2020
b3fbab4
modify : gitignore 수정
giantim Feb 8, 2020
1985f2b
refact : call stack 이 쌓이는 것을 방지하기 위해 잘못된 문자열이 입력될 시 계산기의 기능이 종료되도록 변경…
giantim Feb 8, 2020
061557d
refact : 여러 클래스에서 공통적으로 사용하는 0, 공백, 빈 문자열을 상수 클래스로 분리
giantim Feb 8, 2020
9d2cac8
refact : 변경한 메소드에 맞게 테스트 코드 수정 / MethodSource -> ValueSource 로 변경
giantim Feb 8, 2020
d5d53d5
Merge branch 'onboarding' of https://github.com/giantim/java-calculat…
giantim Feb 8, 2020
4f46d30
refact : Operator 를 컨벤션에 맞게 이름 수정 / Operator.find() 와 Operator.calcul…
giantim Feb 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/main/java/calculator/Input.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
import java.util.Scanner;

public class Input {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Input 이란 클래스명과 inputString, splitString 이란 메서드명으로는, 이 클래스와 메서드가 이 무엇을 입력받고, 무엇을 처리하는지 알기 어렵네요 :)

추상적이거나 광범위한 네이밍보다는 역할과 의도를 드러낼 수 있는 네이밍을 해보는 것이 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

클래스와 메소드의 이름을 짓는 것이 정말 어려운것 같습니다. 왜냐하면 의도를 나타내도록 이름을 짓다보니까 이름이 길어져서 가독성이 떨어지는것 처럼 느껴집니다. 제가 변경한 클래스와 메소드 이름을 보시고 다시 한번 피드백 해주실 수 있나요? :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과거에는 축약어를 사용하는 등으로 코드 양을 줄이려고 했다면, 요즘은 네이밍을 있는 그대로 풀어서 작성하여 가독성을 높이려는 노력을 현업에서 더 많이 하고 있습니다
이름이 길어진다고 가독성이 떨어지진 않습니다. :)

private Validator validator = new Validator();
private ValidityInspector validityInspector = new ValidityInspector();
private Scanner scanner = new Scanner(System.in);

private String inputString() {
System.out.print("계산식을 입력하시오: ");
String inputLine = scanner.nextLine();
try {
validator.isValidInput(inputLine);
validityInspector.isValidInput(inputLine);
} catch (IllegalArgumentException e) {
System.out.println(e.getMessage());
return inputString();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

계속해서 잘못된 문자열을 입력한다면 콜스택이 쌓이게 되겠네요.
out of memory 가 발생할 수 있는 코드는 피하는 것이 좋습니다.
콜스택이 쌓이지 않도록 풀어보는 것은 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 사용자가 정상 문자열을 입력할 때 까지 입력을 계속해서 받도록 하기 위해 try catch 문을 이용하였습니다. 근데 이 방법에서 콜스택이 쌓이는 부분을 고려하지 못했네요. 수정한 방법으로는 사용자가 비정상적인 문자열을 입력하면 계산기가 종료되도록 구현했습니다.

Expand All @@ -22,7 +22,7 @@ public String[] splitString() {
String input = inputString();
String[] splitInput = input.split(" ");
try {
validator.isValidSplitedInput(splitInput);
validityInspector.isValidSplitedInput(splitInput);
} catch (IllegalArgumentException e) {
System.out.println(e.getMessage());
return splitString();
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/calculator/Operator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package calculator;

public enum Operator {
Plus("+"), Minus("-"), Multiplication("*"), Division("/");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum 변수는 대문자로 작성하는 것이 컨벤션입니다 :)


private String symbol;

Operator(String symbol) {
this.symbol = symbol;
}

public String getValue() {
return this.symbol;
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
package calculator;

import java.util.ArrayList;
import java.util.List;

public class Validator {
private static final ArrayList<String> operators = new ArrayList<>();
public class ValidityInspector {
private List<String> operators;

public Validator() {
operators.add("+");
operators.add("-");
operators.add("*");
operators.add("/");
ValidityInspector() {
operators = new ArrayList<>();
operators.add(Operator.Plus.getValue());
operators.add(Operator.Minus.getValue());
operators.add(Operator.Multiplication.getValue());
operators.add(Operator.Division.getValue());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저장해두지 말고 Operator 에게 물어보는 것은 어떨까요?

Operator.contains(String operator)

}

public void isValidInput(String input) {
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/calculator/CalculatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@

public class CalculatorTest {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MethodSource 이 외에도 여러가지 ParameterizedTest 를 위한 Source 들이 제공됩니다 :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MethodSource 로 테스트 코드를 작성했었던 이유는 이전에 테스트를 할 때 Double.toString() 을 이용해서 테스트를 해보려고 했었기 때문이었습니다. 목적에 맞는 Source 를 사용하도록 코드를 변경했습니다. 잘 몰랐던 부분인데 감사합니다 :)

private Calculator calculator;
private Validator validator;
private ValidityInspector validityInspector;

@BeforeEach
public void setUp() {
calculator = new Calculator();
validator = new Validator();
validityInspector = new ValidityInspector();
}

static Stream<String> notValidInputStrings() {
Expand All @@ -30,7 +30,7 @@ static Stream<String> notValidSplitedStrings() {
@MethodSource("notValidInputStrings")
public void isValidInputTest(String notValidInputString) {
Assertions.assertThatThrownBy(() -> {
validator.isValidInput(notValidInputString);
validityInspector.isValidInput(notValidInputString);
}).isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Null or Blank or Empty exception.");
}
Expand All @@ -40,7 +40,7 @@ public void isValidInputTest(String notValidInputString) {
public void isValidSplitedInputTest(String notValidSplitedStrings) {
String[] splitData = notValidSplitedStrings.split(" ");
Assertions.assertThatThrownBy(() -> {
validator.isValidSplitedInput(splitData);
validityInspector.isValidSplitedInput(splitData);
}).isInstanceOf(IllegalArgumentException.class);
}

Expand Down