[문자열 덧셈 계산기] 이나영 미션 제출합니다. #572

Bewheneverwhatiwant · 2024-10-21T14:10:04Z

저는 아래의 절차로 이번 과제를 수행하였습니다.

미션에 필요한 기능들이 무엇일지 생각해보았습니다.
미션에서 사용자가 일으킬 수 있는 예외 상황이 무엇일지 생각해보았습니다.
ApplicationTest.js 파일을 읽으며 테스트코드의 작동 원리를 파악했습니다.
App.js 파일에서 기능 단위로 코드를 작성했습니다.
ApplicationTest.js 파일에서 원하는 테스트케이스를 추가하여 동작을 검증했습니다.
App.js 파일의 함수들을 별도 파일로 분리 후 호출하여 가독성을 높였습니다.
코드 작성 시, 제시된 컨벤션에 유의하였습니다.
커밋 메시지 작성 시, 제시된 컨벤션에 유의하였습니다.

Closes #3

…atiwant/javascript-calculator-7 into bewheneverwhatiwant

Xen-alpha · 2024-10-22T04:33:45Z

입력을 받아 파싱하고 처리한 뒤 출력하는 과정을 단계 별로 모듈화하여 문제가 생겼을 때 고쳐야 할 부분을 파일 단위로 바로 찾을 수 있게 한 구조랑, src/functions/parseInput.js에서 문제가 될 법한 입력을 정규표현식으로 적절한 문자로 replace한 다음 다시 새로운 정규식으로 넣어 구분자와 관련된 예외 처리를 쉽게 할 수 있는 부분이 제 머리를 탁 치게 만드네요!👍👍👍

jaeyoung-kwon

1주차 고생 많으셨습니다!
코드가 워낙 깔끔해서 리뷰 해드릴게 많이 없어보이는데, 부족한 실력으로 리뷰 진행해봤습니다 ㅎㅎ
테스트 짜신 부분도 보고 많이 배워갑니다!

jaeyoung-kwon · 2024-10-23T01:43:36Z

src/functions/addFuntions/validateNumber.js

+
+    if (isNaN(parsedNum)) {
+        throw new Error('[ERROR] 숫자가 아닌 값이 포함되었습니다.');
+    } else if (parsedNum < 0) {


첫 if에서 숫자가 아닐 경우 error로 던져지기 때문에 else를 알써도 되지 않을까 생각합니다!

헉 맞네요 ...! 다음부터는 필요없는 코드는 아닐지 한번 더 숙고하고 코드를 짜야겠어요 !

jaeyoung-kwon · 2024-10-23T01:45:38Z

src/functions/add.js

+import { parseInput } from './addFuntions/parseInput.js';
+import { validateNumber } from './addFuntions/validateNumber.js';
+
+export function add(input) {


add라는 함수에 input을 parse하고 그것들을 split해서 더하는 것도 진행되고 있는 것 같은데, 한 함수에 add만 하는 것이 아니라 다른 기능도 여러 개 들어간 것 같습니다. 단일 책임 원칙으로 함수를 더 쪼개서 함수에 대한 직관성을 높이면 어떨까 생각이 듭니다!

너무 좋은 피드백이네요 ! 제가 아직 초보라 입력/계산/출력 정도로만 나눠도 가독성이 좋지 않을까? 라는 막연한 생각을 가졌었는데, 단일 책임의 원칙에 대해서 공부하고 2주차 미션에서는 신경써야겠어요. 조언 너무 감사드립니다 !!

jaeyoung-kwon · 2024-10-23T01:46:48Z

src/App.js

 class App {
-  async run() {}
+  async run() {


저는 try-catch로 에러 핸들링하는 것을 생각 못했는데 보고 배워갑니다 ㅎㅎ

제가 매 코드마다 try-catch를 재탕(?)하는 버릇이 있어서...ㅎㅎ 이 구조가 가장 보기좋다는 인식이 있어서 바로 적용할 수 있었어요 !! 칭찬해주셔서 너무 감사드립니다 🍀

jaeyoung-kwon님 혹시 woowa 저장소 풀리퀘 주소 남겨주시면 제가 거기서 코드리뷰 해드릴 수 있을 것 같아요 !! 이 답글 보신다면 PR 번호나 주소 남겨주시면 맞리뷰 해드리구 싶습니다 😄

Bewheneverwhatiwant · 2024-10-23T04:11:16Z

입력을 받아 파싱하고 처리한 뒤 출력하는 과정을 단계 별로 모듈화하여 문제가 생겼을 때 고쳐야 할 부분을 파일 단위로 바로 찾을 수 있게 한 구조랑, src/functions/parseInput.js에서 문제가 될 법한 입력을 정규표현식으로 적절한 문자로 replace한 다음 다시 새로운 정규식으로 넣어 구분자와 관련된 예외 처리를 쉽게 할 수 있는 부분이 제 머리를 탁 치게 만드네요!👍👍👍

안녕하세요 !! 중간고사 이슈로 소중한 리뷰를 이제 확인했네요 ㅠㅠ 파일 분리와 예외 처리에 신경쓴 걸 알아봐주셔서 너무 기뻐요 ! 저도 xena님 코드 구경하러 가겠습니다 👯

Bewheneverwhatiwant · 2024-10-23T04:19:41Z

입력을 받아 파싱하고 처리한 뒤 출력하는 과정을 단계 별로 모듈화하여 문제가 생겼을 때 고쳐야 할 부분을 파일 단위로 바로 찾을 수 있게 한 구조랑, src/functions/parseInput.js에서 문제가 될 법한 입력을 정규표현식으로 적절한 문자로 replace한 다음 다시 새로운 정규식으로 넣어 구분자와 관련된 예외 처리를 쉽게 할 수 있는 부분이 제 머리를 탁 치게 만드네요!👍👍👍

xena 님! woowa 저장소에 풀리퀘 주소나 번호 알려주시면 맞리뷰 해드리구 싶어요 !! 이 답글 보신다면 꼭 회신 주셔요 !! 😆

chaeseungyun

멋진 코드 구경하고 많이 배우고 갑니다! README도 예뻐요

chaeseungyun · 2024-10-23T05:42:22Z

src/App.js

+    } catch (error) {
+      Console.print(`${error.message}`);
+      throw error; // 예외를 다시 던져서 테스트에서 잡히도록 처리
+    }


간결한 run 함수 너무 깔끔하네요!
저는 어차피 에러를 처리하지 못하고 throw 하므로 try, catch는 굳이 사용하지 않아도 된다고 생각했어요
에러 객체를 throw 하기때문에 에러 메시지도 콘솔에 출력하지 않아도 될 것 같아요

아...! 제가 생각치 못했던 부분이네요 ! 깔끔함과 익숙함에 취해서 '이유'를 생각하지 않은 코딩이었던 것 같아요 ㅠㅠ 2주차 미션에서는 더 고민해서 App.js 구조를 작성해보겠습니다 !!

chaeseungyun · 2024-10-23T05:57:05Z

src/functions/addFuntions/parseInput.js

+        input = parts[1];
+    }
+
+    return { numbers: input, delimiter }; // 파싱된 값 반환


delimiter와 같은 상수는 외부에 정의해 두는게 관리하기 좋을 것 같습니다

파라미터를 바꾸는 것은 위험할 수 있다고 생각합니다! 새로운 변수에 할당하는게 안전해보여요

커스텀 구분자가 여러 글자여도 괜찮은가 보네요? 저랑은 다르게 해석하셔서 재밌게 봤습니다!

좋은 지적 너무 감사드립니다 ! 2번은 제가 '수정해야지 ~~' 생각만 해두고 수정을 못해버렸네요 ㅠㅠ 그리구 커스텀 구분자가 여러 글자여도 괜찮게 한 것이 맞아욤 !! 2개 이상의 특수문자를 붙일 경우 하나로 묶여서 구분자로 사용 가능하도록 예외를 대처했습니다 !

chaeseungyun · 2024-10-23T05:58:02Z

src/functions/add.js

+    const parsedNumbers = numbers
+        .split(delimiter)
+        .filter(num => num !== '') // 중복된 구분자 처리
+        .map(validateNumber); // 숫자 유효성 검증 및 변환


로직 깔끔해용~
하지만 저도 함수를 조금 더 분리하면 좋을 것 같다는 생각입니다!

add 함수에 너무 많은 역할이 있는 것 말씀이시죠 ? 다음 미션에서는 단일 책임의원칙에 대해 더 공부하고 코딩하려구요 !! 좋은 지적 너무 감사드려요 🍀

goodbyebada

부족한 실력이지만 코드 리뷰 남기고 갑니다 ㅎㅎ
커밋에 docs 개행 수정 메시지가 보이는데 커밋 합치기에 대해 사용하시면 더 좋을 것 같아요!
저도 수정을 커밋 메시지에 남겼었는데 스쿼시를 통해 해결했더니 기능단위로 보여서 편하더라구요!
2주차도 같이 화이팅해요!

goodbyebada · 2024-10-23T06:37:43Z

src/functions/add.js

+        .map(validateNumber); // 숫자 유효성 검증 및 변환
+
+    return parsedNumbers.reduce((sum, num) => sum + num, 0); // 합산
+}


저도 마찬가지로 add라는 함수안에 있는 작업들을 분리하면 후에 유지 보수할때 더 좋을 것 같아요!

commit rebase로 합치지 못한 것이 아쉽다고 생각하고 있었는데 !! 좋은 지적해주세서 감사해요 🍀 스쿼시는 사용 경험이 없지만 다음 미션에서 도전해보겠습니다 !

goodbyebada · 2024-10-23T06:49:46Z

src/functions/addFuntions/parseInput.js

+        input = parts[1];
+    }
+
+    return { numbers: input, delimiter }; // 파싱된 값 반환


parts[0] parts[1] 대신
const [numbers, delimeter] = parts
혹은
const [numbers, delimeter] = put.split('\\n');와 같이 명시적인 네이밍을 사용한다면 가독성이 더 좋을 것 같아요!

모든 커스텀 문자열을 \$&으로 replace 하셨는데, \$&이 어떤 의미인지 궁금합니다! \으로 replace 하신걸까요?

맞네요 ! 좀 더 명확한 네이밍을 하는 걸 습관들여야겠습니다 !

. $&는 JS의 string.prototype.replace()에서 사용하는 특수치환인데, "매칭된 문자열 자체를 반환" 하는 기능을 해요 !
정규식이 구분자 문자열에서 특수문자를 찾으면, replace() 메서드가 특수문자 앞에 \를 붙여요.
이 과정에서 $&가 매칭된 특수문자 자체를 의미하기 때문에, 문자가 무엇이든간에 그대로 유지하고 앞에 \를 붙일 수 있어요 !
저는 예외 케이스에서 커스텀 구분자가 , /, \n, 두개인 경우 등 다양한 경우를 고려했기때문에,
정확하게 특수문자를 파싱해오는게 중요하다고 생각했고
$&는 커스텀 구분자가 정규식에서 제대로 처리되는 것을 보장하기 위해 사용했습니다 ㅎㅎ

goodbyebada · 2024-10-23T06:56:58Z

src/functions/add.js

+    const { numbers, delimiter } = parseInput(input); // 입력 파싱
+    const parsedNumbers = numbers
+        .split(delimiter)
+        .filter(num => num !== '') // 중복된 구분자 처리


여러개의 구분자가 있다는 케이스는 생각을 못했었는데 배워갑니다 ㅎㅎ

Xen-alpha · 2024-10-23T09:27:31Z

입력을 받아 파싱하고 처리한 뒤 출력하는 과정을 단계 별로 모듈화하여 문제가 생겼을 때 고쳐야 할 부분을 파일 단위로 바로 찾을 수 있게 한 구조랑, src/functions/parseInput.js에서 문제가 될 법한 입력을 정규표현식으로 적절한 문자로 replace한 다음 다시 새로운 정규식으로 넣어 구분자와 관련된 예외 처리를 쉽게 할 수 있는 부분이 제 머리를 탁 치게 만드네요!👍👍👍

xena 님! woowa 저장소에 풀리퀘 주소나 번호 알려주시면 맞리뷰 해드리구 싶어요 !! 이 답글 보신다면 꼭 회신 주셔요 !! 😆

Pull Request #34 입니다! 리뷰 부탁드려요~

sung-yeop

여러모로 저와 다르게 작성된 부분이 많아서 더욱 흥미롭게 코드 리뷰를 할 수 있었습니다
저는 제가 가지고 있는 생각들을 하나씩 정리해보면서 아이디어를 공유하고 싶었어요

언제든지 피드백은 환영입니다
좋은 코드 잘 봤습니다!

sung-yeop · 2024-10-23T16:05:54Z

src/functions/addFuntions/parseInput.js


-        if (parts.length < 2 || parts[1].trim() === "") {
+        if (parts.length < 2 || parts[1].trim() === '') {
            throw new Error('[ERROR] 잘못된 입력 형식입니다.');
        }



구분자를 판별하는 과정에서 정규표현식을 사용해보면 어떨까요?
if 조건문을 중첩해서 사용하는 것보다 훨씬 간단하게 처리할 수 있다고 생각합니다

맞네요 그럼 저 부분이 좀 더 깔끔해질 것 같아요 !! 조언 감사드려요 ! 👍

sung-yeop · 2024-10-23T16:14:50Z

src/App.js

      const customDelimiter = parts[0].slice(2).replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
-      delimiter = new RegExp(`${customDelimiter}`);
+      delimiter = RegExp(`[${customDelimiter},:]`);


백틱을 사용해서 정규표현식을 구분하려는 시도는 저도 해봤는데, 제 예상과는 자꾸 다른 결과가 나오더라구요..

customDelimiter를 추출하는 과정에서 match 메서드를 이용해보는건 어떨까요?
저 같은 경우에는 아래와 같이 사용했습니다. '어떤게 더 좋은 방법이다'라고 하는게 아닌, 시야를 넓히는 느낌으로 한번 봐주시면 될 것 같아요!
저두 slice, replace를 이용해서 customDelimiter를 추출하는 부분은 배워갑니다

const customDeli = /\/\/(\D+)\\n/.test(input) ? input.match(/\/\/(\D+)\\n/)[1] : "";

맞아요 보니깐 다른 분들도 match()를 많이 사용하셨더라구요 ! 다음 미션에서 분발해보겠습니다 !! 👍

sung-yeop · 2024-10-23T16:17:17Z

__tests__/ApplicationTest.js

+    outputs.forEach((output) => {
+      expect(logSpy).toHaveBeenCalledWith(expect.stringContaining(output));
+    });
+  });


제가 이번 과제에서 미처 고려하지 못했던 부분이 바로 테스트 케이스를 작성하는 부분이었습니다
많은 분들이 제출해주신 코드를 보면서 테스트코드를 엄청 열심히 작성해주신 부분을 보고 동기부여를 받고갑니다!!

sung-yeop · 2024-10-23T16:24:36Z

src/add.js

+        });
+
+    return numbers.reduce((sum, num) => sum + num, 0);
+}


마지막에 로직을 분리하는 커밋을 보면서 디렉토리 구조에 신경쓰는 것 같다는 생각이 들었어요.
App.js에서 run 메서드에 연산 관련 로직이 순서대로 들어가있는 것처럼 보입니다
별도의 객체를 생성해서 객체의 기능을 따로 묶어두는건 어떨까요?
물론 이번 과제에서는 계산기에서 단순히 덧셈 기능만 있기 때문에 꼭 필요하다고는 생각하지 않습니다.
하지만, 객체로 분리해서 기능들을 관리하는 방식이 유지보수나 확장성 측면에서 더 유용할 것이라는 생각이 들어요

물론 저도 객체의 기능을 어디까지 결정해야할지는 아직 고민이 많지만 제가 가지고 있는 생각을 말씀드려봤습니다. 이 내용이 하나의 아이디어가 되었으면 좋겠어요!

정말 좋은 지적이세요 !! 다른 분들의 코드를 봐도, 저처럼 하신 분들도 계시지만 객체지향적으로 코드를 작성하신 불들이 많으시더라고요. 파일을 둘러봐도, OOP 스럽게 쓰신 코드는 읽기 편하고 다른 케이스를 적용하기도 좋아보였어요. 다음 미션에서는 더 OOP 스러운 코드 작성을 시도하겠습니다 !!

mindaaaa

미션 진행하시느라 고생 많으셨습니다! 다음 미션도 같이 화이팅해봐요:)

mindaaaa · 2024-10-23T20:24:38Z

src/functions/addFuntions/parseInput.js

+    if (input.startsWith('//')) {
+        const parts = input.split('\\n');
+
+        if (parts.length < 2 || parts[1].trim() === '') {


저는 정규표현식을 사용하지 않고 문제 해결하기 까다롭다고 생각했는데, 이런 식으로 접근할 수도 있군요! 많이 배워갑니다.

윗분께서 말씀주신대로 조건문이 좀 중첩되는 느낌이 있어서, 다음 미션에서는 더 깔끔하게 해보려구요 !! 칭찬 감사드립니다 👍

mindaaaa · 2024-10-23T20:28:05Z

src/functions/addFuntions/validateNumber.js

+        throw new Error('[ERROR] 양수로만 이루어져야 합니다.');
+    }
+
+    return parsedNum;


구체적으로 잘 해결한 좋은 코드라고 생각합니다!
다만 변수나 함수 이름이 조금 더 명시적이고 구체적이면 더욱 가독성 있는 멋진 코드가 나올 것 같습니다!

피드백 감사합니다 !! 변수명과 함수명에서 의도를 바로 알 수 있도록 네이밍하는 습관을 들이도록 할게요 !! 👍

Bewheneverwhatiwant added 30 commits October 18, 2024 00:41

docs: README 문구 추가

6f0da34

docs: REACME 내용 보완 - branch 전략 추가

f2f9c42

docs: README 브랜치 전략 수정

d2b9264

feat: 빈 문자열 입력 시 0 반환 기능 추가

fe7259c

Closes #3

docs: README 내용 추가 - 이슈 자동닫힘->수동닫음으로 전략 변경

249df5a

feat: 쉼표구분자 문자열 입력 시 연산 기능 추가

316cc8f

docs: README 일부 내용 삭제

e1020e0

docs: README 개행 수정

7dabd65

feat: 쉼표 또는 콜론 구분자의 문자열 입력 시 연산 기능 추가

7abe52f

feat: 커스텀 구분자 지원 기능 추가

4ed98a0

feat: 음수, 숫자가 아닌 값, 잘못된입력 처리 기능 추가

f526e61

docs: README에 사용자 입력 겨우의 수 내용 추가

c2100f7

docs: README 개행 분리

a36fc48

refactor: run() 내부 try catch로 수정, 컨벤션에 따른 수정

614215e

feat: 커스텀 구분자와 기본 구분자를 혼용해도 연산 가능하도록 기능 추가

08968bb

docs: README 기능 및 사용자 경우의 수 추가

3b9b3f9

chore(ApplicationTest): 테스트케이스(쉼표 구분자)추가

a87d260

chore(ApplicationTest): 테스트케이스(zhffhs rnqnswk) 추가

8068e41

chore(ApplicationTest): 테스트케이스(콜론 구분자) 추가

ca88274

Merge branch 'bewheneverwhatiwant' of https://github.com/Bewheneverwh…

41788bd

…atiwant/javascript-calculator-7 into bewheneverwhatiwant

chore(ApplicationTest): 테스트케이스(커스텀 구분자만 입력하는 경우) 추가

bc4ca74

chore(ApplicationTest): 테스트케이스(커스텀 구분자가 /,\,\n인 경우) 추가

865940c

style: AirBnB 가이드에 맞춰 큰따옴표를 작은따옴표로 변경

1560c40

style(App): AirBnB 가이드에 맞춰 큰따옴표를 작은따옴표로 변경

62aec6b

Merge branch 'bewheneverwhatiwant' of https://github.com/Bewheneverwh…

ed58de9

…atiwant/javascript-calculator-7 into bewheneverwhatiwant

refactor(App): 3개의 함수들을 별도 파일로 분리

ebb788a

refactor(all): 파일 구조를 폴더로 정리

acf275e

refactor(add): 2개 함수를 별도 파일로 분리

6975470

docs(README): 프로그램 흐름을 정리한 내용 추가

5578dc8

docs(README): 오타 및 개행 수정

a32249e

Bewheneverwhatiwant added 6 commits October 21, 2024 04:14

docs(README): 디렉토리 구조 설명 추가

fc27c21

docs(README): 개행 수정

7127a59

docs(README): 기능들의 상위 기능 정의

8db452d

docs(README): 개행 수정

540ce83

docs(README): 개행 수정

593ae60

refactor(all): eslint.config.js 파일에 따른 eslint적용

a651bc2

jaeyoung-kwon reviewed Oct 23, 2024

View reviewed changes

chaeseungyun reviewed Oct 23, 2024

View reviewed changes

goodbyebada reviewed Oct 23, 2024

View reviewed changes

sung-yeop reviewed Oct 23, 2024

View reviewed changes

mindaaaa reviewed Oct 23, 2024

View reviewed changes

[문자열 덧셈 계산기] 이나영 미션 제출합니다. #572

Are you sure you want to change the base?

[문자열 덧셈 계산기] 이나영 미션 제출합니다. #572

Conversation

Bewheneverwhatiwant commented Oct 21, 2024

Xen-alpha commented Oct 22, 2024

jaeyoung-kwon 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

Bewheneverwhatiwant commented Oct 23, 2024

Bewheneverwhatiwant commented Oct 23, 2024

chaeseungyun 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

goodbyebada 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

Xen-alpha commented Oct 23, 2024

sung-yeop 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

mindaaaa 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