-
Notifications
You must be signed in to change notification settings - Fork 0
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
base: main
Are you sure you want to change the base?
Changes from 30 commits
dd4b867
dac220d
9c52493
2e7cd6d
0fe1bba
dedece8
061b121
fc10e29
2fc1483
3a094f5
5f2ea86
b9b2316
fdb0c7c
a033f18
bef0412
93e9eaa
0ea8127
000c1f8
338341c
1f14323
50c64a7
93954de
a9ee173
48eaf2d
c7a6f7f
5e7a559
955ccb6
358957c
2af965b
ba18632
31920dd
71f7fed
e9e5596
f5514b5
9a99a54
40b3489
8451b22
98313ec
5897fa6
0ea7745
a4d6f60
ce6ed73
831a58f
c145fc0
c677162
9ed071b
f4a9c3d
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 |
---|---|---|
@@ -1 +1,39 @@ | ||
# java-calculator-precourse | ||
# java-calculator-precourse | ||
|
||
## 구현할 기능 목록 | ||
|
||
### 1. 문자열을 입력 받는 기능 | ||
|
||
### 2. 문자열에 추가 구분자 입력이 있는지 확인하는 기능 | ||
|
||
### 3. 문자열을 구분자를 기준으로 나누는 기능 | ||
|
||
### 4. 구분자로 나눠진 문자열에 숫자 외의 다른 문자가 있는지 확인하는 기능 있으면 IllegalArgumentException | ||
|
||
### 5. 나눠진 숫자를 더하는 기능 | ||
|
||
## 테스트 필요 하다 생각 되는 것들 | ||
|
||
- [x] 문자열을 아무 것도 주지 않았을 때 | ||
- [x] 추가 구분자로 숫자를 줬을 때 | ||
- [x] 소수 넣어 보기(추가 구분자로 .을 줬을 때와 아닐 때) | ||
- [x] 역슬래시(이스케이프 문자를 구분자로 줬을 때) | ||
- [x] 구분자 사이에 숫자가 없을 때 | ||
|
||
## 확인 필요 | ||
|
||
### 1. 커스텀 구분자가 여러 개라면? | ||
현재 만들어진 로직은 커스텀 구분자가 한 개인 경우만을 받고 있으나 그렇지 않을 수도 있음. | ||
Comment on lines
+40
to
+43
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. 1번은 진짜 생각도 못 했네요 ㄷㄷㄷ 만약 커스텀 구분자가 여러 개라면 |
||
|
||
### 2. 커스텀 구분자에 숫자가 들어온다면? | ||
당연히 계산기라면 커스텀 구분자에 숫자가 들어오면 안되긴 함. | ||
커스텀 구분자에 숫자가 들어오는 경우 IllegalArgumentException 해야하는가? | ||
아니면 구분자로 인식하고 계산시켜야 하는가? | ||
지금은 계산하도록 만들어둠. | ||
|
||
### 3. 아무 입력도 하지 않았다면? | ||
입력이 없으면 0을 출력하도록 했음. 근데 IllegalArgumentException 해야하는지? | ||
|
||
### 4. 구분자 사이에 숫자가 없다면? | ||
현재는 사이에 숫자가 없다면 0으로 인식하고 계산을 진행하도록 하였음. | ||
이것도 throw 해야하는 경우인가? |
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 = new Configuration(); | ||
|
||
public static void main(String[] args) { | ||
// TODO: 프로그램 구현 | ||
CalculatorController calculatorController = configuration.getCalculatorController(); | ||
calculatorController.input(); | ||
} | ||
} |
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.
단순히 만약 후자의 역할이라면, 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.
@holyPigeon 후자의 역할로 생각하고 만든 클래스긴 합니다! Configuration 을 추가적으로 만들지 못하도록 구현을 해봐야겠네요. 방금 찾아봤는데 그런걸 Singleton pattern 이라고 디자인 패턴에 있네요 하나 배워갑니당 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package calculator.config; | ||
|
||
import calculator.controller.CalculatorController; | ||
import calculator.service.CalculatorService; | ||
|
||
public class Configuration { | ||
private final CalculatorController calculatorController; | ||
|
||
public Configuration() { | ||
this.calculatorController = new CalculatorController(new CalculatorService()); | ||
} | ||
|
||
public CalculatorController getCalculatorController() { | ||
return calculatorController; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package calculator.controller; | ||
|
||
import calculator.service.CalculatorService; | ||
import camp.nextstep.edu.missionutils.Console; | ||
|
||
public class CalculatorController { | ||
private final CalculatorService calculatorService; | ||
|
||
public CalculatorController(CalculatorService calculatorService) { | ||
this.calculatorService = calculatorService; | ||
} | ||
|
||
public void input() { | ||
System.out.println("덧셈할 문자열을 입력해 주세요."); | ||
String s = Console.readLine(); | ||
output(s); | ||
} | ||
|
||
private void output(String s) { | ||
int sum = calculatorService.sum(s); | ||
System.out.println("결과 : " + sum); | ||
} | ||
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. 저 같은 경우는 입출력 기능을 따로 클래스로 만들어 분리해봤는데, 이 부분도 한 번 고려해보시면 좋을 것 같습니다! 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.
ㄷㄷㄷㄷ 제가 그정도로 치밀하질 못했네요 |
||
|
||
} |
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. 현재 CalculatorService에서 원본 문자열에 대한 파싱 작업을 같이 하고 계신 것으로 보입니다! 저 같은 경우에는 계산 기능과 문자열 가공 기능을 다른 클래스로 분리해봤습니다. 아무래도 우리가 실제 계산기를 사용할 때 문자열이 아닌 숫자만을 입력하는 것처럼, 계산기의 본 기능은 "계산"이고, "계산을 위해 숫자를 분리해내는 일"은 다른 클래스가 가져야 할 책임이라고 생각했기 떄문입니다! 살짝 참고해주시면 좋을 것 같습니다...! 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. 딱 외부에서 사용할 메서드만 public으로 선언하시고, 나머지 하위 메서드들은 private으로 선언하신 게 인상깊네요! 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.
동감합니다..! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
package calculator.service; | ||
|
||
public class CalculatorService { | ||
private String s; | ||
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 String plusSeparator; | ||
|
||
/** | ||
* 덧셈 로직을 순차적으로 진행한다. | ||
*/ | ||
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. 메서드 하나하나 |
||
public int sum(String s) { | ||
this.s = s; | ||
boolean hasPlusSeparator = checkPlusSeparator(); | ||
String[] separatedString = separate(hasPlusSeparator); | ||
hasNaN(separatedString); | ||
return sumSeparatedStringArr(separatedString); | ||
} | ||
|
||
/** | ||
* 커스텀 구분자가 있는지 확인한다. | ||
* | ||
* @return 만약 커스텀 구분자가 있다면 true, 없다면 false 를 반환한다. | ||
*/ | ||
private boolean checkPlusSeparator() { | ||
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. 처음엔 주석을 통해 커스텀 구분자 확인에 관한 메서드라는 것을 알 수 있었네요. 사소한 부분이지만 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.
맞아요 ㅋㅋㅋ... 처음에 만들때 메서드 이름을 저렇게 정하고 나서 바꿀 생각을 못했네욤.. 메서드랑 변수 이름 바꾸러 갑니당.. |
||
if (s.length() >= 5 && s.startsWith("//") && s.startsWith("\\n", 3)) { | ||
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.
|
||
plusSeparator = String.valueOf(s.charAt(2)); | ||
s = s.substring(5); | ||
return true; | ||
} | ||
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. 저는 이에 따라 전 너무 디테일하게 들어가는 걸 수도 있긴 한데 한 번 고민 정도는 해보셔도 좋을 것 같아요! |
||
return false; | ||
} | ||
|
||
/** | ||
* 받은 문자열을 구분자를 기준으로 나눈다. 만약 커스텀 구분자에 역슬래시가 있다면 문제가 생기지 않도록 한다. | ||
* | ||
* @param hasPlusSeparator 커스텀 구분자가 있다면 true, 없다면 false 를 입력한다. | ||
* @return 분리된 문자열을 반환한다. 이 문자열에는 반드시 숫자만 있는 것은 아니다. | ||
*/ | ||
private String[] separate(boolean hasPlusSeparator) { | ||
if (!hasPlusSeparator) { | ||
return s.split("[:,]"); | ||
} | ||
if (plusSeparator.equals("\\")) { | ||
return s.split("[:," + plusSeparator.repeat(2) + "]"); | ||
} | ||
return s.split("[:," + plusSeparator + "]"); | ||
} | ||
|
||
/** | ||
* 숫자가 아닌 문자열이 있는지 확인한다. 만약 있다면 IllegalArgumentException 으로 처리한다. | ||
* | ||
* @param separatedStringArr 숫자 외의 문자가 있는지 확인하고 싶은 문자열을 입력한다. | ||
*/ | ||
private void hasNaN(String[] separatedStringArr) { | ||
for (String s : separatedStringArr) { | ||
for (char c : s.toCharArray()) { | ||
if (c < 48 || c > 57) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} | ||
} | ||
} | ||
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문 안에 if문이 들어가서 indent depth가 3이 된 것 같습니다! 한 눈에 이해하기 조금 어려운 면이 있어서 좀 더 직관적으로 리팩토링할 방법이 있나? 찾아보시면 좋을 것 같아요! 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.
아래에 따로 메서드를 만들 생각을 했었는데 한 메서드에서 사용할 메서드를 따로 만드는게 맞나 생각했어요. 그래도 가독성을 생각하면 따로 만드는게 맞겠죠? 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. 일단 1차적으로 대부분의 중첩 for문들이 stream문으로 바꿨을 떄 더 간결해지긴 합니다! 저라면 이렇게 바꿨을 것 같습니다. private void hasNaN(String[] separatedStringArr) {
if (Arrays.stream(separatedStringArr)
.flatMapToInt(String::chars) // 각 문자열을 문자 스트림으로 변환
.anyMatch(c -> c < '0' || c > '9')) { // 숫자가 아닌 문자가 있는지 확인
throw new IllegalArgumentException();
}
} 여기서 if문의 조건절이 너무 길어지는 부분이 있어서, |
||
|
||
/** | ||
* 문자열을 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
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. for문 대신 stream문을 사용할 수도 있습니다! 지금은 간단한 로직이라 별 차이가 안 나지만, 나중에 for문 안에 있는 로직이 복잡해지면 stream 사용을 고려해보셔도 좋을 것 같아요 😀 private int sumSeparatedStringArr(String[] separatedStringArr) {
return Arrays.stream(separatedStringArr)
.filter(s -> !s.isEmpty()) // 빈 문자열 필터링
.mapToInt(Integer::parseInt) // 문자열을 정수로 변환
.sum(); // 합계 계산
} 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.
가독성도 그렇고 stream 이 직관적인 면이 있어서 보기 이쁘다고 생각을 해요. 코틀린에서는 stream 문이 워낙 편해서 자주 썼었는데 자바는 좀 손에 안익더라고요... 그래도 써보려고 노력은 해봐야죠. 조언 감사합니다! |
||
} |
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. 꼼꼼한 테스트 작성 아름답습니다...! 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.
ㅋㅋㅋㅋㅋ 감사합니다! 제가 잡생각이 많아서 코드 짜면서 생각나는 것들 다 해보다 보니까 저렇게 됐네용 |
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.
예외 케이스를 되게 다양하게 짜신 것 같습니다 👍
특히
소수 넣어 보기
같은 경우는 생각을 더 해보게 되는 것 같아요. 덧셈 계산기라는 특성상 추후 발전을 시킨다면 소수도 계산을 할 수 있어야 할텐데, 이 때 어떤 기준으로 소수를 분리할 지에 대해 고민이 되네요!좋은 생각거리를 주셔서 감사합니다 🔥