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

whrjsgml0000 · 2024-10-19T04:40:01Z

No description provided.

holyPigeon

커밋도 되게 많이 작성하시고 코드도 꼼꼼하게 짜신 것 같습니다 ㄷㄷㄷ
고생하셨습니다!!!

holyPigeon · 2024-10-19T05:50:39Z

README.md

+- [x] 추가 구분자로 숫자를 줬을 때
+- [x] 소수 넣어 보기(추가 구분자로 .을 줬을 때와 아닐 때)
+- [x] 역슬래시(이스케이프 문자를 구분자로 줬을 때)
+- [x] 구분자 사이에 숫자가 없을 때


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

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

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

holyPigeon · 2024-10-19T05:53:40Z

README.md

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


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

holyPigeon · 2024-10-19T06:01:52Z

src/main/java/calculator/config/Configuration.java

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

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

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

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

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

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

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

holyPigeon · 2024-10-19T06:03:07Z

src/main/java/calculator/controller/CalculatorController.java

+    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);
+    }


저 같은 경우는 입출력 기능을 따로 클래스로 만들어 분리해봤는데, 이 부분도 한 번 고려해보시면 좋을 것 같습니다!

저 같은 경우는 입출력 기능을 따로 클래스로 만들어 분리해봤는데, 이 부분도 한 번 고려해보시면 좋을 것 같습니다!

ㄷㄷㄷㄷ 제가 그정도로 치밀하질 못했네요

holyPigeon · 2024-10-19T06:09:43Z

src/main/java/calculator/service/CalculatorService.java

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

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

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

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

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

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

holyPigeon · 2024-10-19T06:12:10Z

src/main/java/calculator/service/CalculatorService.java

+    /**
+     * 커스텀 구분자가 있는지 확인한다.
+     *
+     * @return 만약 커스텀 구분자가 있다면 true, 없다면 false 를 반환한다.
+     */
+    private boolean checkPlusSeparator() {


처음엔 checkPlustSeperator 라는 메서드명을 듣고 의미를 잘 모르겠어서 조금 갸우뚱한 부분이 있었습니다!

주석을 통해 커스텀 구분자 확인에 관한 메서드라는 것을 알 수 있었네요. 사소한 부분이지만 PlusSeparator보다 CustomSeparator가 더 직관적인 이름이지 않을까 싶어요!

처음엔 checkPlustSeperator 라는 메서드명을 듣고 의미를 잘 모르겠어서 조금 갸우뚱한 부분이 있었습니다!

주석을 통해 커스텀 구분자 확인에 관한 메서드라는 것을 알 수 있었네요. 사소한 부분이지만 PlusSeparator보다 CustomSeparator가 더 직관적인 이름이지 않을까 싶어요!

맞아요 ㅋㅋㅋ... 처음에 만들때 메서드 이름을 저렇게 정하고 나서 바꿀 생각을 못했네욤.. 메서드랑 변수 이름 바꾸러 갑니당..

holyPigeon · 2024-10-19T06:19:38Z

src/main/java/calculator/service/CalculatorService.java

+        if (s.length() >= 5 && s.startsWith("//") && s.startsWith("\\n", 3)) {
+            plusSeparator = String.valueOf(s.charAt(2));
+            s = s.substring(5);
+            return true;
+        }


저는 커스텀 구분자가 반드시 한 글자가 아닐수도 있겠다라는 생각을 했어요. 이런 가정을 덧붙이면, s.startsWith("\\n", 3) 조건의 경우 구분자가 두 글자 이상이 될 때 에러를 일으킬 수 있습니다.

이에 따라 전 s.indexOf("//") + 2, s.indexOf("\\n") 를 통해 커스텀 구분자 선언부의 시작과 끝을 설정하고서 커스텀 구분자를 추출해냈습니다.

너무 디테일하게 들어가는 걸 수도 있긴 한데 한 번 고민 정도는 해보셔도 좋을 것 같아요!

holyPigeon · 2024-10-19T06:23:02Z

src/main/java/calculator/service/CalculatorService.java

+    private void hasNaN(String[] separatedStringArr) {
+        for (String s : separatedStringArr) {
+            for (char c : s.toCharArray()) {
+                if (c < 48 || c > 57) {
+                    throw new IllegalArgumentException();
+                }
+            }
+        }
+    }


2중 for문 안에 if문이 들어가서 indent depth가 3이 된 것 같습니다! 한 눈에 이해하기 조금 어려운 면이 있어서 좀 더 직관적으로 리팩토링할 방법이 있나? 찾아보시면 좋을 것 같아요!

2중 for문 안에 if문이 들어가서 indent depth가 3이 된 것 같습니다! 한 눈에 이해하기 조금 어려운 면이 있어서 좀 더 직관적으로 리팩토링할 방법이 있나? 찾아보시면 좋을 것 같아요!

아래에 따로 메서드를 만들 생각을 했었는데 한 메서드에서 사용할 메서드를 따로 만드는게 맞나 생각했어요. 그래도 가독성을 생각하면 따로 만드는게 맞겠죠?

일단 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문의 조건절이 너무 길어지는 부분이 있어서, boolean containNonDigit(String[] separatedStringArr) 이런 느낌으로 2차적으로 메서드를 분리했을 것 같아요 😀

holyPigeon · 2024-10-19T06:25:20Z

src/main/java/calculator/service/CalculatorService.java

+    private int sumSeparatedStringArr(String[] separatedStringArr) {
+        int sum = 0;
+        for (String separatedString : separatedStringArr) {
+            if (!separatedString.isEmpty()) {
+                sum += Integer.parseInt(separatedString);
+            }
+        }
+        return sum;
+    }


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

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

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

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

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

holyPigeon · 2024-10-19T06:26:09Z

src/test/java/calculator/ApplicationTest.java

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

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

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

SongJaeHoonn · 2024-10-20T05:29:34Z

src/main/java/calculator/service/CalculatorService.java

+import java.util.Arrays;
+
+public class CalculatorService {
+    private String s;


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

SongJaeHoonn · 2024-10-20T05:36:20Z

src/main/java/calculator/service/CalculatorService.java

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


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

SongJaeHoonn · 2024-10-20T05:39:27Z

src/main/java/calculator/service/CalculatorService.java

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


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

SongJaeHoonn · 2024-10-20T05:41:16Z

src/main/java/calculator/service/CalculatorService.java

+        if (Arrays.stream(separatedStringArr)
+                .flatMapToInt(String::chars)
+                .anyMatch(it -> it < '0' || it > '9')) {
+            throw new IllegalArgumentException();


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

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

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

whrjsgml0000 and others added 29 commits October 15, 2024 23:15

docs:README.md update

dd4b867

feat:문자열 입력 기능 추가

dac220d

feat:Calculator 클래스 추가, checkPlusSeparator() 메서드 추가

9c52493

docs:update README.md

2e7cd6d

feat:separate 메서드 추가

0fe1bba

docs:update README.md

dedece8

docs:update README.md

061b121

feat:hasNaN 메서드 추가

fc10e29

docs:update README.md

2fc1483

feat:sum 메서드 추가

3a094f5

style:clean code

5f2ea86

refactor:separate 메소드 else 문 제거

b9b2316

comment:Calculator 클래스에 주석 추가

fdb0c7c

test:add test cases

a033f18

fix:문자열을 아무 것도 주지 않았을 때 예외 처리 추가

bef0412

refactor:디ë레이어 기반으로 ë디렉토리 나누는 중.

93e9eaa

refactor:코드 옮기기 끝

0ea8127

fix:ë¨merge conflict í� 해결

000c1f8

주석 달기

338341c

docs:update README.md

1f14323

fix:구분자 사이에 문자열이 없는 경우 처리 추가

50c64a7

test:구분자 사이에 문자열이 없는 경우 테스트 추가

93954de

refactor:메인에 있던 코드 컨트롤러로 옮김.

a9ee173

refactor:input 메서드와 output 메서드로 기능 분리

48eaf2d

style:코드 정리

c7a6f7f

test:테스트 케이스 추가

5e7a559

docs:Update README.md

955ccb6

comment:주석 한글화함.

358957c

refator:불필요한 else 문 삭제

2af965b

whrjsgml0000 self-assigned this Oct 19, 2024

docs:Update README.md

ba18632

holyPigeon reviewed Oct 19, 2024

View reviewed changes

whrjsgml0000 added 4 commits October 19, 2024 18:53

2중 for 문 제거. stream 사용

31920dd

docs:Update README.md

71f7fed

refactor:Configuration 클래스에 싱글톤 패턴 적용

e9e5596

refactor:CalculatorService 클래스의 sum() -> add() 로 변경

f5514b5

whrjsgml0000 force-pushed the whrjsgml0000 branch from a1c35db to f5514b5 Compare October 20, 2024 03:13

whrjsgml0000 added 3 commits October 20, 2024 12:16

refactor:CalculatorController 클래스의 변수이름 변경

9a99a54

refactor:view를 따로 관리하도록 분리함

40b3489

refactor:사용하지 않는 import 문 제거

8451b22

SongJaeHoonn reviewed Oct 20, 2024

View reviewed changes

whrjsgml0000 added 8 commits October 20, 2024 17:19

refactor:오류 메세지 작성

98313ec

refactor:변수 이름 변경

5897fa6

refactor:CalculatorValidator 클래스 생성 및 로직 분리

0ea7745

refactor:상수 관리 enum 클래스 생성

a4d6f60

refactor:변수 이름 변경

ce6ed73

refactor:클래스 이름 변경 및 그에 따른 변수 이름 변경

831a58f

refactor:Separator 클래스 생성 및 로직 분리

c145fc0

comment:주석 추가

c677162

whrjsgml0000 force-pushed the whrjsgml0000 branch from 8863740 to c677162 Compare October 20, 2024 10:52

whrjsgml0000 added 2 commits October 21, 2024 01:14

docs:Update README.md

9ed071b

docs:Update README.md

f4a9c3d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

whrjsgml0000 commented Oct 19, 2024

holyPigeon left a comment

holyPigeon Oct 19, 2024

holyPigeon Oct 19, 2024

holyPigeon Oct 19, 2024

whrjsgml0000 Oct 19, 2024

holyPigeon Oct 19, 2024

whrjsgml0000 Oct 19, 2024

holyPigeon Oct 19, 2024

holyPigeon Oct 19, 2024

SongJaeHoonn Oct 20, 2024

holyPigeon Oct 19, 2024

whrjsgml0000 Oct 19, 2024

holyPigeon Oct 19, 2024

holyPigeon Oct 19, 2024

whrjsgml0000 Oct 19, 2024

holyPigeon Oct 19, 2024

holyPigeon Oct 19, 2024

whrjsgml0000 Oct 19, 2024

holyPigeon Oct 19, 2024

whrjsgml0000 Oct 19, 2024

SongJaeHoonn Oct 20, 2024

SongJaeHoonn Oct 20, 2024

SongJaeHoonn Oct 20, 2024

SongJaeHoonn Oct 20, 2024

whrjsgml0000 Oct 20, 2024


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

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

Are you sure you want to change the base?

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

Conversation

whrjsgml0000 commented Oct 19, 2024

holyPigeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment