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

[LBP] 송하은 미션 3,4단계 제출합니다. #72

Open
wants to merge 6 commits into
base: haeunsong
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ repositories {

dependencies {
testImplementation platform('org.junit:junit-bom:5.9.1')
testImplementation platform('org.assertj:assertj-bom:3.25.1')
testImplementation('org.junit.jupiter:junit-jupiter')
testImplementation('org.assertj:assertj-core')
}

test {
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/Calculator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
public class Calculator {

public int add(int a, int b) {
return a + b;
}

public int subtract(int a, int b) {
return a - b;
}

public int multiply(int a, int b) {
return a * b;
}

public int divide(int a, int b) {
if (b == 0) {
throw new ArithmeticException("Divide by zero");
}
return a / b;
}
}
53 changes: 53 additions & 0 deletions src/main/java/StringCalculator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import java.util.regex.Pattern;

public class StringCalculator {

public int getSum(String str) {
Copy link

Choose a reason for hiding this comment

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

하나의 메서드가 너무 많은 책임을 가지고 있어요
코드를 단번에 이해하기에는 어려웠어요 좀 더 분리해볼까요?

Copy link
Author

Choose a reason for hiding this comment

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

  1. 입력값 검증
  2. 구분자 추출
  3. 문자열을 구분자 기준으로 분리
  4. 분리된 숫자 문자열 배열을 합산
    으로 분리해보았습니다!

Copy link

Choose a reason for hiding this comment

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

좀 더 의미있는 역할과 의도에 맞는 변수로 네이밍 해봅시다


if(str.length() == 0) {
return 0;
}
int sum = 0;
String separater = "[,|:]";
String numbers = str;
Copy link

Choose a reason for hiding this comment

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

매개변수로 넘어온 str을 어떤 의도로 numbers라는 변수로 대입했나요?
아래 코드를 쭉 살펴보았을때도 의문만 늘어나는 것 같은데 하은이의 의도를 알려주세요

Copy link
Author

Choose a reason for hiding this comment

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

현재 코드를 보면, numbers 는 마지막 커스텀 구분자인 \n 이후의 값들을 뜻합니다. 만약 numbers 를 "" 으로 두면,

String[] tokens = numbers.split(separater);
        if(tokens.length == 1) {
            return 0;
        }

기본 구분자인 경우에는 올바르게 사용하지 못하기에 numbers에 str 을 대입하였습니다.

하지만, 여전히 헷갈리는 부분이 존재하여 알려주신 피드백 반영하여 새로 코드를 작성해보았습니다!


if (str.startsWith("//") && str.contains("\n")) {

int newLineIdx = str.indexOf("\n");
// /n 을 찾을 수 없거나, \n 이후 값이 없는 경우
if (newLineIdx == -1) {
throw new RuntimeException("잘못된 입력 형식입니다.");
Copy link

Choose a reason for hiding this comment

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

예외 사항을 정확하게 전달해보면 어떨까요?

}
separater = Pattern.quote(str.substring(2, newLineIdx));
Copy link

Choose a reason for hiding this comment

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

정규식 특수문자를 안전하게 처리하셨네요! 👍

numbers = str.substring(newLineIdx + 1); // \n 다음부터 끝까지 숫자가 있는 부분들

// \n 이후에 최소한 구분자가 한 개 이상은 있어야한다. 구분자가 없다면 배열 길이가 1이된다.
if(numbers.isEmpty() || numbers.split(separater).length == 1) {
throw new RuntimeException("잘못된 입력 형식입니다.");
}
Copy link

Choose a reason for hiding this comment

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

해당 조건문에서 "//;\n2"처럼 한 개의 숫자만 입력된 경우 예외가 발생할 가능성이 있어보여요
"2"는 정상적인 입력으로 볼 수 있어야하죠 다시 개선해봅시다!

  • "구분자가 하나 이상 포함되어야 한다"는 규칙은 요구사항에 없음

}
// 구분자를 기준으로 split 진행 후 sum 실행
String[] tokens = numbers.split(separater);
if(tokens.length == 1) {
return 0;
}
for (String token : tokens) {
int num = parseNumber(token);
if (num < 0) {
throw new RuntimeException("음수는 사용할 수 없습니다.");
}
sum += num;
}

return sum;
}

public int parseNumber(String number) {
// 숫자가 아니라면 예외 발생
try {
return Integer.parseInt(number);
} catch (NumberFormatException e) {
throw new RuntimeException("숫자가 아닌 값이 포함되어 있습니다.");
}
}
Copy link

Choose a reason for hiding this comment

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

외부에서 호출할 필요없는 메서드는 접근제어자를 통해서 캡슐화를 지켜봅시다!

}
48 changes: 48 additions & 0 deletions src/test/java/CalculatorTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

@DisplayName("초간단 계산기 테스트")
class CalculatorTest {
private final Calculator calculator = new Calculator();

@Nested
class Add {
@Test
void add() {
assertEquals(5, calculator.add(2, 3));
}
}

@Nested
class Subtract {
@Test
void subtract() {
assertEquals(-3, calculator.subtract(3, 6));
}
}

@Nested
class Multiply {
@Test
void multiply() {
assertEquals(6, calculator.multiply(2, 3));
}
}

@Nested
class Divide{
@Test
@DisplayName("0으로 나누면 ArithmeticException 예외가 발생한다.")
void should_throw_exception_when_divide_by_zero() {
assertThrows(ArithmeticException.class, () -> calculator.divide(3,0));
}

@Test
void divideByNotZero() {
assertEquals(2, calculator.divide(6, 3));
}
}
}
89 changes: 89 additions & 0 deletions src/test/java/StringCalculatorTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

@DisplayName("문자열 계산기 테스트")
class StringCalculatorTest {

StringCalculator stringCalculator = new StringCalculator();
Copy link

Choose a reason for hiding this comment

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

해당 클래스에서만 사용된다면 인스턴스 변수 또한 private로 캡슐화를 지켜봅시다


@Nested
Copy link

Choose a reason for hiding this comment

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

@Nested를 통해서 테스트를 그룹화 하셨네요 좋습니다!

@DisplayName("기본 구분자 테스트")
class basicSeparaterTest {
@Test
@DisplayName("빈 문자열이면 0을 반환해야 한다.")
void should_return_0_when_string_is_empty() {
assertThat(0).isEqualTo(stringCalculator.getSum(""));
}
@Test
@DisplayName("구분자가 없는 경우 0을 반환해야 한다.")
void test() {
assertThat(0).isEqualTo(stringCalculator.getSum("123"));
}
@Test
@DisplayName("쉼표(,) 또는 콜론(:)으로 구분된 숫자들의 합을 반환해야 한다.")
void should_sum_numbers_separated_by_comma_or_colon() {
assertThat(6).isEqualTo(stringCalculator.getSum("1,2:3"));
}
}

@Nested
@DisplayName("커스텀 구분자 테스트")
class customSeparaterTest {
@Test
@DisplayName("커스텀 구분자로 분리한 숫자들의 합을 반환해야 한다")
void should_sum_numbers_separated_by_custom_separator() {
assertThat(6).isEqualTo(stringCalculator.getSum("//;\n1;2;3"));
}
}

@Nested
@DisplayName("Runtime Exception 테스트")
class runtimeExceptionTest {
@Test
@DisplayName("숫자가 아닌 값이 포함되면 예외를 발생해야 한다.")
void should_throw_exception_when_contains_non_number() {

// 특수문자가 포함된 경우
assertThatThrownBy(() -> stringCalculator.getSum("//;\n1;*;3"))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("숫자가 아닌 값이 포함되어 있습니다.");

assertThatThrownBy(() -> stringCalculator.getSum("//;\n1456;-;3"))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("숫자가 아닌 값이 포함되어 있습니다.");

// 글자가 포함된 경우
assertThatThrownBy(() -> stringCalculator.getSum("//;\nddd;ad;3"))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("숫자가 아닌 값이 포함되어 있습니다.");
}
Copy link

Choose a reason for hiding this comment

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

@Parameterized, @CsvSource에 대해서 학습해보고
중복되는 코드를 줄이고 가독성 올려봅시다!

Copy link
Author

Choose a reason for hiding this comment

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

오 이러한 것들이 있군요. 알려주셔서 감사합니다. 반영해보겠습니다!


@Test
@DisplayName("음수가 포함되면 예외를 발생해야 한다.")
void should_throw_exception_when_contains_minus() {
Copy link

Choose a reason for hiding this comment

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

다른 메서드들과 컨벤션이 맞춰지지 않은 것 같아요
메소드 시그니처 라인과의 개행을 하은이만의 기준으로 통일해봅시다!


assertThatThrownBy(() -> stringCalculator.getSum("-4,2:-9"))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("음수는 사용할 수 없습니다.");
}

@Test
@DisplayName("또 다른 잘못된 입력 형식의 경우 예외를 발생해야 한다.")
void should_throw_exception_when_another_wrong_input() {

assertThatThrownBy(() -> stringCalculator.getSum("//;1;2;\n;"))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("잘못된 입력 형식입니다.");

assertThatThrownBy(() -> stringCalculator.getSum("//;1;2;\n123"))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("잘못된 입력 형식입니다.");

}
}

Comment on lines +79 to +82
Copy link

Choose a reason for hiding this comment

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

불필요한 개행들이 보입니다 제거해볼까요?

}