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

[문자열 덧셈 계산기] 조건희 미션 제출합니다 #2

Open
wants to merge 47 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
dd4b867
docs:README.md update
whrjsgml0000 Oct 15, 2024
dac220d
feat:문자열 입력 기능 추가
whrjsgml0000 Oct 15, 2024
9c52493
feat:Calculator 클래스 추가, checkPlusSeparator() 메서드 추가
whrjsgml0000 Oct 15, 2024
2e7cd6d
docs:update README.md
whrjsgml0000 Oct 15, 2024
0fe1bba
feat:separate 메서드 추가
whrjsgml0000 Oct 15, 2024
dedece8
docs:update README.md
whrjsgml0000 Oct 15, 2024
061b121
docs:update README.md
whrjsgml0000 Oct 15, 2024
fc10e29
feat:hasNaN 메서드 추가
whrjsgml0000 Oct 15, 2024
2fc1483
docs:update README.md
whrjsgml0000 Oct 15, 2024
3a094f5
feat:sum 메서드 추가
whrjsgml0000 Oct 15, 2024
5f2ea86
style:clean code
whrjsgml0000 Oct 16, 2024
b9b2316
refactor:separate 메소드 else 문 제거
whrjsgml0000 Oct 16, 2024
fdb0c7c
comment:Calculator 클래스에 주석 추가
whrjsgml0000 Oct 16, 2024
a033f18
test:add test cases
whrjsgml0000 Oct 16, 2024
bef0412
fix:문자열을 아무 것도 주지 않았을 때 예외 처리 추가
whrjsgml0000 Oct 16, 2024
93e9eaa
refactor:디ë레이어 기반으로 ë디렉토리 나누는 중.
whrjsgml0000 Oct 17, 2024
0ea8127
refactor:코드 옮기기 끝
whrjsgml0000 Oct 17, 2024
000c1f8
fix:ë¨merge conflict í• 해결
whrjsgml0000 Oct 17, 2024
338341c
주석 달기
whrjsgml0000 Oct 17, 2024
1f14323
docs:update README.md
whrjsgml0000 Oct 17, 2024
50c64a7
fix:구분자 사이에 문자열이 없는 경우 처리 추가
whrjsgml0000 Oct 17, 2024
93954de
test:구분자 사이에 문자열이 없는 경우 테스트 추가
whrjsgml0000 Oct 17, 2024
a9ee173
refactor:메인에 있던 코드 컨트롤러로 옮김.
whrjsgml0000 Oct 19, 2024
48eaf2d
refactor:input 메서드와 output 메서드로 기능 분리
whrjsgml0000 Oct 19, 2024
c7a6f7f
style:코드 정리
whrjsgml0000 Oct 19, 2024
5e7a559
test:테스트 케이스 추가
whrjsgml0000 Oct 19, 2024
955ccb6
docs:Update README.md
whrjsgml0000 Oct 19, 2024
358957c
comment:주석 한글화함.
whrjsgml0000 Oct 19, 2024
2af965b
refator:불필요한 else 문 삭제
whrjsgml0000 Oct 19, 2024
ba18632
docs:Update README.md
whrjsgml0000 Oct 19, 2024
31920dd
2중 for 문 제거. stream 사용
whrjsgml0000 Oct 19, 2024
71f7fed
docs:Update README.md
whrjsgml0000 Oct 19, 2024
e9e5596
refactor:Configuration 클래스에 싱글톤 패턴 적용
whrjsgml0000 Oct 19, 2024
f5514b5
refactor:CalculatorService 클래스의 sum() -> add() 로 변경
whrjsgml0000 Oct 20, 2024
9a99a54
refactor:CalculatorController 클래스의 변수이름 변경
whrjsgml0000 Oct 20, 2024
40b3489
refactor:view를 따로 관리하도록 분리함
whrjsgml0000 Oct 20, 2024
8451b22
refactor:사용하지 않는 import 문 제거
whrjsgml0000 Oct 20, 2024
98313ec
refactor:오류 메세지 작성
whrjsgml0000 Oct 20, 2024
5897fa6
refactor:변수 이름 변경
whrjsgml0000 Oct 20, 2024
0ea7745
refactor:CalculatorValidator 클래스 생성 및 로직 분리
whrjsgml0000 Oct 20, 2024
a4d6f60
refactor:상수 관리 enum 클래스 생성
whrjsgml0000 Oct 20, 2024
ce6ed73
refactor:변수 이름 변경
whrjsgml0000 Oct 20, 2024
831a58f
refactor:클래스 이름 변경 및 그에 따른 변수 이름 변경
whrjsgml0000 Oct 20, 2024
c145fc0
refactor:Separator 클래스 생성 및 로직 분리
whrjsgml0000 Oct 20, 2024
c677162
comment:주석 추가
whrjsgml0000 Oct 20, 2024
9ed071b
docs:Update README.md
whrjsgml0000 Oct 20, 2024
f4a9c3d
docs:Update README.md
whrjsgml0000 Oct 20, 2024
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
42 changes: 41 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,41 @@
# java-calculator-precourse
# java-calculator-precourse

## 구현할 기능 목록

### 1. 문자열을 입력 받는 기능

### 2. 문자열에 추가 구분자 입력이 있는지 확인하는 기능

### 3. 문자열을 구분자를 기준으로 나누는 기능

### 4. 구분자로 나눠진 문자열에 숫자 외의 다른 문자가 있는지 확인하는 기능 있으면 IllegalArgumentException

### 5. 나눠진 숫자를 더하는 기능

## 테스트 필요 하다 생각 되는 것들

- [x] 문자열을 아무 것도 주지 않았을 때
- [x] 추가 구분자로 숫자를 줬을 때
- [x] 소수 넣어 보기(추가 구분자로 .을 줬을 때와 아닐 때)
- [x] 역슬래시(이스케이프 문자를 구분자로 줬을 때)
- [x] 구분자 사이에 숫자가 없을 때
Comment on lines +34 to +38

Choose a reason for hiding this comment

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

예외 케이스를 되게 다양하게 짜신 것 같습니다 👍

특히 소수 넣어 보기 같은 경우는 생각을 더 해보게 되는 것 같아요. 덧셈 계산기라는 특성상 추후 발전을 시킨다면 소수도 계산을 할 수 있어야 할텐데, 이 때 어떤 기준으로 소수를 분리할 지에 대해 고민이 되네요!

좋은 생각거리를 주셔서 감사합니다 🔥


## 확인 필요

### 1. 커스텀 구분자가 여러 개라면?
현재 만들어진 로직은 커스텀 구분자가 한 개인 경우만을 받고 있으나 그렇지 않을 수도 있음.
Comment on lines +40 to +43

Choose a reason for hiding this comment

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

1번은 진짜 생각도 못 했네요 ㄷㄷㄷ 만약 커스텀 구분자가 여러 개라면 //;\n//-\n1-2;3 이런 식으로 문자열 앞 쪽에 선언부를 여러 개 써야하지 않을까?라는 생각이 듭니다!

만약에 //와 \n 사이에 두 개가 들어가면 그건 따로 떨어뜨려서 구분을 해야하나?
//ea\n1ea2ea3ea 이 주어지면 ea로 묶어서 구분자로 사용해야 하나?

### 2. 커스텀 구분자에 숫자가 들어온다면?
당연히 계산기라면 커스텀 구분자에 숫자가 들어오면 안되긴 함.
커스텀 구분자에 숫자가 들어오는 경우 IllegalArgumentException 해야하는가?
아니면 구분자로 인식하고 계산시켜야 하는가?
지금은 계산하도록 만들어둠.

### 3. 아무 입력도 하지 않았다면?
입력이 없으면 0을 출력하도록 했음. 근데 IllegalArgumentException 해야하는지?

### 4. 구분자 사이에 숫자가 없다면?
현재는 사이에 숫자가 없다면 0으로 인식하고 계산을 진행하도록 하였음.
이것도 throw 해야하는 경우인가?
8 changes: 7 additions & 1 deletion src/main/java/calculator/Application.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package calculator;

import calculator.config.Configuration;
import calculator.controller.CalculatorController;

public class Application {
private static final Configuration configuration = Configuration.getInstance();

public static void main(String[] args) {
// TODO: 프로그램 구현
CalculatorController calculatorController = configuration.getCalculatorController();
calculatorController.input();
}
}
19 changes: 19 additions & 0 deletions src/main/java/calculator/config/Configuration.java

Choose a reason for hiding this comment

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

Configuration 클래스의 역할이 궁금합니다!

단순히 CalculatorServiceCalculatorController를 한꺼번에 초기화해주는 역할인지, 아니면 final 키워드를 통해 컨트롤러를 하나의 인스턴스로 관리하는 역할인지 궁금합니다.

만약 후자의 역할이라면, Configuration 클래스 자체는 새로운 인스턴스를 계속 생성할 수 있기 때문에 의도가 불투명해질 수도 있겠다는 생각이 듭니다...!

Copy link
Author

Choose a reason for hiding this comment

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

Configuration 클래스의 역할이 궁금합니다!

단순히 CalculatorServiceCalculatorController를 한꺼번에 초기화해주는 역할인지, 아니면 final 키워드를 통해 컨트롤러를 하나의 인스턴스로 관리하는 역할인지 궁금합니다.

만약 후자의 역할이라면, Configuration 클래스 자체는 새로운 인스턴스를 계속 생성할 수 있기 때문에 의도가 불투명해질 수도 있겠다는 생각이 듭니다...!

@holyPigeon 후자의 역할로 생각하고 만든 클래스긴 합니다! Configuration 을 추가적으로 만들지 못하도록 구현을 해봐야겠네요. 방금 찾아봤는데 그런걸 Singleton pattern 이라고 디자인 패턴에 있네요 하나 배워갑니당

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package calculator.config;

import calculator.controller.CalculatorController;

public class Configuration {
private static final Configuration configuration = new Configuration();
private final CalculatorController calculatorController = new CalculatorController();

private Configuration() {
}

public static Configuration getInstance() {
return configuration;
}

public CalculatorController getCalculatorController() {
return calculatorController;
}
}
29 changes: 29 additions & 0 deletions src/main/java/calculator/controller/CalculatorController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package calculator.controller;

import calculator.service.CalculatorService;
import calculator.view.InputView;
import calculator.view.OutputView;

public class CalculatorController {
private final CalculatorService calculatorService;
private final InputView inputView;
private final OutputView outputView;

public CalculatorController() {
this.inputView = new InputView();
this.outputView = new OutputView();
this.calculatorService = new CalculatorService();
}

public void input() {
String s = inputView.inputString();
inputView.close();
int addedResult = calculatorService.add(s);
output(addedResult);
}

private void output(int i) {
outputView.outputInteger(i);
}

}
78 changes: 78 additions & 0 deletions src/main/java/calculator/service/CalculatorService.java

Choose a reason for hiding this comment

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

현재 CalculatorService에서 원본 문자열에 대한 파싱 작업을 같이 하고 계신 것으로 보입니다!

저 같은 경우에는 계산 기능과 문자열 가공 기능을 다른 클래스로 분리해봤습니다. 아무래도 우리가 실제 계산기를 사용할 때 문자열이 아닌 숫자만을 입력하는 것처럼, 계산기의 본 기능은 "계산"이고, "계산을 위해 숫자를 분리해내는 일"은 다른 클래스가 가져야 할 책임이라고 생각했기 떄문입니다!

살짝 참고해주시면 좋을 것 같습니다...!

Choose a reason for hiding this comment

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

딱 외부에서 사용할 메서드만 public으로 선언하시고, 나머지 하위 메서드들은 private으로 선언하신 게 인상깊네요!

Choose a reason for hiding this comment

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

계산기의 본 기능은 "계산"이고, "계산을 위해 숫자를 분리해내는 일"은 다른 클래스가 가져야 할 책임이라고 생각했기 떄문입니다!

동감합니다..!
한 클래스가 많은 책임을 가지면 확장성 및 일관성을 저해할 수 있어요.
추후 유지보수를 위해 기능을 나눠 다른 클래스로 책임을 분리하는게 좋아보입니다!
그게 객체지향이 가지는 큰 장점이라고 생각합니다😀

Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package calculator.service;

import java.util.Arrays;

public class CalculatorService {
private String s;

Choose a reason for hiding this comment

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

s라는 필드의 의미가 코드를 처음 보는 사람에게는 헷갈릴 수 있을 것 같아요.
의미상 input의 의미인 것 같은데, s라는 필드명 보다는 확실한 변수명으로 작성하는게 좋아보입니다

private String customSeparator;

/**
* 덧셈 로직을 순차적으로 진행한다.
*/

Choose a reason for hiding this comment

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

메서드 하나하나 Java Docs를 사용하신 점이 좋은 것 같아요. 계산 관련 서비스라 로직이 복잡할 수 밖에 없는데,
설명이 친절하게 적혀있으니 이해하기 쉽네요!!

public int add(String s) {
this.s = s;
boolean hasCustomSeparator = checkCustomSeparator();
String[] separatedString = separate(hasCustomSeparator);
hasNaN(separatedString);
return sumSeparatedStringArr(separatedString);
}

/**
* 커스텀 구분자가 있는지 확인한다.
*
* @return 만약 커스텀 구분자가 있다면 true, 없다면 false 를 반환한다.
*/
private boolean checkCustomSeparator() {
if (s.length() >= 5 && s.startsWith("//") && s.startsWith("\\n", 3)) {

Choose a reason for hiding this comment

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

5, 3, 2과 같은 상수를 처리하는 부분을 enum으로 관리하면 추후 요구사항이 바뀌더라도 한 번에 처리할 수 있고, 해당 상수가 이름을 가지므로 어떤 의미를 가지는지 확실히 알 수 있기 때문에 이해하기도 더 쉬워질 것 같아요

customSeparator = String.valueOf(s.charAt(2));
s = s.substring(5);
return true;
}
return false;
}

/**
* 받은 문자열을 구분자를 기준으로 나눈다. 만약 커스텀 구분자에 역슬래시가 있다면 문제가 생기지 않도록 한다.
*
* @param hasCustomSeparator 커스텀 구분자가 있다면 true, 없다면 false 를 입력한다.
* @return 분리된 문자열을 반환한다. 이 문자열에는 반드시 숫자만 있는 것은 아니다.
*/
private String[] separate(boolean hasCustomSeparator) {
if (!hasCustomSeparator) {
return s.split("[:,]");
}
if (customSeparator.equals("\\")) {
return s.split("[:," + customSeparator.repeat(2) + "]");
}
return s.split("[:," + customSeparator + "]");
}

/**
* 숫자가 아닌 문자열이 있는지 확인한다. 만약 있다면 IllegalArgumentException 으로 처리한다.
*
* @param separatedStringArr 숫자 외의 문자가 있는지 확인하고 싶은 문자열을 입력한다.
*/
private void hasNaN(String[] separatedStringArr) {
if (Arrays.stream(separatedStringArr)
.flatMapToInt(String::chars)
.anyMatch(it -> it < '0' || it > '9')) {
throw new IllegalArgumentException();

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.

효과적인 에러 핸들링을 위해 예외에 메시지를 끼워넣는 것은 어떠신가요?

그게 좋아보이네요! 감사합니다!

}
}

/**
* 문자열을 int 형으로 바꾼 뒤 합한다.
*
* @param separatedStringArr 숫자로만 이루어진 문자열 배열을 입력한다.
* @return 합한다.
*/
private int sumSeparatedStringArr(String[] separatedStringArr) {
int sum = 0;
for (String separatedString : separatedStringArr) {
if (!separatedString.isEmpty()) {
sum += Integer.parseInt(separatedString);
}
}
return sum;
}
Comment on lines +31 to +39

Choose a reason for hiding this comment

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

for문 대신 stream문을 사용할 수도 있습니다! 지금은 간단한 로직이라 별 차이가 안 나지만, 나중에 for문 안에 있는 로직이 복잡해지면 stream 사용을 고려해보셔도 좋을 것 같아요 😀

private int sumSeparatedStringArr(String[] separatedStringArr) {
    return Arrays.stream(separatedStringArr)
            .filter(s -> !s.isEmpty())       // 빈 문자열 필터링
            .mapToInt(Integer::parseInt)     // 문자열을 정수로 변환
            .sum();                          // 합계 계산
}

Copy link
Author

Choose a reason for hiding this comment

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

for문 대신 stream문을 사용할 수도 있습니다! 지금은 간단한 로직이라 별 차이가 안 나지만, 나중에 for문 안에 있는 로직이 복잡해지면 stream 사용을 고려해보셔도 좋을 것 같아요 😀

private int sumSeparatedStringArr(String[] separatedStringArr) {
    return Arrays.stream(separatedStringArr)
            .filter(s -> !s.isEmpty())       // 빈 문자열 필터링
            .mapToInt(Integer::parseInt)     // 문자열을 정수로 변환
            .sum();                          // 합계 계산
}

가독성도 그렇고 stream 이 직관적인 면이 있어서 보기 이쁘다고 생각을 해요. 코틀린에서는 stream 문이 워낙 편해서 자주 썼었는데 자바는 좀 손에 안익더라고요... 그래도 써보려고 노력은 해봐야죠. 조언 감사합니다!

}
14 changes: 14 additions & 0 deletions src/main/java/calculator/view/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package calculator.view;

import camp.nextstep.edu.missionutils.Console;

public class InputView {
public String inputString() {
System.out.println("덧셈할 문자열을 입력해주세요.");
return Console.readLine();
}

public void close() {
Console.close();
}
}
7 changes: 7 additions & 0 deletions src/main/java/calculator/view/OutputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package calculator.view;

public class OutputView {
public void outputInteger(int i) {
System.out.println("결과 : " + i);
}
}
72 changes: 67 additions & 5 deletions src/test/java/calculator/ApplicationTest.java

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.

꼼꼼한 테스트 작성 아름답습니다...!

ㅋㅋㅋㅋㅋ 감사합니다! 제가 잡생각이 많아서 코드 짜면서 생각나는 것들 다 해보다 보니까 저렇게 됐네용

Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package calculator;

import camp.nextstep.edu.missionutils.test.NsTest;
import org.junit.jupiter.api.Test;

import static camp.nextstep.edu.missionutils.test.Assertions.assertSimpleTest;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import camp.nextstep.edu.missionutils.test.NsTest;
import org.junit.jupiter.api.Test;

class ApplicationTest extends NsTest {
@Test
void 커스텀_구분자_사용() {
Expand All @@ -19,13 +19,75 @@ class ApplicationTest extends NsTest {
@Test
void 예외_테스트() {
assertSimpleTest(() ->
assertThatThrownBy(() -> runException("-1,2,3"))
.isInstanceOf(IllegalArgumentException.class)
assertThatThrownBy(() -> runException("-1,2,3"))
.isInstanceOf(IllegalArgumentException.class)
);
}

@Override
public void runMain() {
Application.main(new String[]{});
}

@Test
public void 숫자_구분자_사용() {
assertSimpleTest(() -> {
run("//1\\n2:213");
assertThat(output()).contains("결과 : 7");
});
}

@Test
public void 탈출_문자_사용() {
assertSimpleTest(() -> {
assertThatThrownBy(() -> runException("1\\2:3"))
.isInstanceOf(IllegalArgumentException.class);
});
}

@Test
public void 탈출_문자_구분자_사용() {
assertSimpleTest(() -> {
run("//\\\\n1\\2:3,4");
assertThat(output()).contains("결과 : 10");
});
}

@Test
public void 마침표_구분자_사용() {
assertSimpleTest(() -> {
run("//.\\n1.2.4:3");
assertThat(output()).contains("결과 : 10");
});
}

@Test
public void 마침표_사용(){
assertThatThrownBy(() -> runException("1.2.3:4"))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
public void 따옴표_구분자_사용() {
assertSimpleTest(() -> {
run("//\"\\n1\"2\"4:3");
assertThat(output()).contains("결과 : 10");
});
}

@Test
public void 구분자_사이에_문자열_없음() {
assertSimpleTest(() -> {
run("1,:2");
assertThat(output()).contains("결과 : 3");
});
}

@Test
public void 문자열_없음(){
assertSimpleTest(() -> {
run("\n");
assertThat(output()).contains("결과 : 0");
});
}
}