Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 8 additions & 3 deletions src/main/java/calculator/controller/CalculatorController.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@

public class CalculatorController {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

컨트롤러에서 뷰와 모델을 static call을 하는 이유가 있나요?
특별한 이유가 없다면 의존성을 생각해서 DI를 하는게 더 낫지 않을까요?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

아하 그 방향대로 수정해보겠습니다!

public void startCalculator() {
String input = InputView.getInput();
int result = Calculator.processSum(input);
OutputView.printResult(result);
try {
String input = InputView.getInput();
int result = Calculator.processSum(input);
OutputView.printResult(result);
} catch (IllegalArgumentException e) {
OutputView.printError(e.getMessage());
throw e;
}
}
}
19 changes: 17 additions & 2 deletions src/main/java/calculator/model/Calculator.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

덕분에 좋은 라이브러리들 알아갑니다

public class Calculator {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

필드가 없고 기능만 있다면 모델 보다는 service나 utils로 취급되는게 더 적절해보여요!
모델로 둔다면 모델인 만큼 상태를 저장하는 목적 등이 강조되면 좀 더 좋을 것 같아요

public static int processSum(String numbers) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이 메서드가 갖고있는 책임이 너무 큰 것 같습니다. 구분자를 확인 및 설정하는 것, 예외를 처리하는 것, 계산하는 것 등 세부 기능으로 메서드를 만들어 분리하는 쪽이 더 좋지 않을까요?

Expand All @@ -22,12 +21,28 @@ public static int processSum(String numbers) {
numbers = matcher.group(2);
}

if (numbers.trim().isEmpty()) {
throw new IllegalArgumentException("커스텀 구분자 이후 숫자가 없습니다.");
}

String delimiterRegex = String.join("|", delimiters);

return Arrays.stream(numbers.split(delimiterRegex))
.map(String::trim)
.filter(num -> !num.isEmpty())
.mapToInt(Integer::parseInt)
.mapToInt(Calculator::parseValidation)
.sum();
}

private static int parseValidation(String num) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

밑에서 변환 예외와 음수 예외를 다루는데 제가 받아들이기에는 역할이 2개여서 가독성이 떨어지는 것 같아요. 개인적인 생각으로는 검증 역시 메서드를 나눠도 좋을 것 같아요.
그리고 검증을 모델 내부에 두신 이유도 궁금합니다!ㅎㅎ

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

검증이랑 예외처리를 따로 분리하고싶었으나...시간 부족이슈로.. 구현과 테스트 정상작동에 집중했습니다. 수정해보겠습니다!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

어떠한 기능만 수행해서 return해주는 메서드의 파라미터는 첫 글자만 남겨서 네이밍해도 괜찮을 것 같아요.
ex) number -> n, string -> s

Copy link
Copy Markdown
Author

@seulnan seulnan Feb 12, 2025

Choose a reason for hiding this comment

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

제안해주신 방식도 간결한 코드에서 효과적일 수 있다고도 생각합니다. 다만, 네이밍이 짧아지면 가독성이 떨어지고 유지보수할 때 혼란이 될 수도 있다고 생각해서요 그래도 현록님이 이 방식이 더 괜찮다고 생각하시는 이유가 있을까용? 생각하시는 더 좋은 규칙이 있다면 같이 얘기해보고싶어요! @Regyung

try {
int value = Integer.parseInt(num);
if (value < 0) {
throw new IllegalArgumentException("음수는 입력할 수 없습니다: " + num);
}
return value;
} catch (NumberFormatException e) {
throw new IllegalArgumentException("잘못된 숫자 입력: " + num);
}
}
}
2 changes: 1 addition & 1 deletion src/main/java/calculator/view/InputView.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

public class InputView {
public static String getInput() {
System.out.print("문자열을 입력하세요:");
System.out.print("문자열을 입력하세요 :");
String userInputValue = Console.readLine();

return userInputValue;
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/calculator/view/OutputView.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

public class OutputView {
public static void printResult(int result) {
System.out.println("결과: " + result);
System.out.println("결과 : " + result);
}
public static void printError(String message) {
System.out.println("오류 발생 : " + message);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

저도 같은 생각입니다!

}
}
14 changes: 7 additions & 7 deletions src/test/java/calculator/ApplicationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ class ApplicationTest extends NsTest {
});
}

// @Test
// void 예외_테스트() {
// assertSimpleTest(() ->
// assertThatThrownBy(() -> runException("-1,2,3"))
// .isInstanceOf(IllegalArgumentException.class)
// );
// }
@Test
void 예외_테스트() {
assertSimpleTest(() ->
assertThatThrownBy(() -> runException("-1,2,3"))
.isInstanceOf(IllegalArgumentException.class)
);
}

@Override
public void runMain() {
Expand Down