Skip to content

Conversation

@whitecity01
Copy link

@whitecity01 whitecity01 commented Oct 19, 2025

기능 목록

1️⃣ 입력 및 출력

  • Console 모듈을 활용하여 입력(readLine)과 출력(print) 기능을 구현.

2️⃣ 커스텀 구분자 파싱

  • 문자열 앞부분에서 커스텀 구분자(최대 1회)를 인식 및 추출.

  • 파싱 후 기본 구분자 리스트에 추가하고, 본문 문자열에서 제거 처리.

3️⃣ 숫자 분리

  • 기본 및 커스텀 구분자를 이용해 문자열 내 숫자를 분리.

  • 각 문자를 순회하며 숫자 단위로 조합 후 Number로 변환 및 배열에 저장.

4️⃣ 유효성 검증 및 에러 처리

  • NaN, 0, 음수, 비숫자 입력 시 throw new Error()로 예외 처리.

  • 에러 발생 시 상위에서 별도 처리 없이 프로그램 종료.

5️⃣ 숫자 합산

  • 검증 통과 시 reduce()를 통해 모든 숫자의 합을 계산 후 반환.

Copy link

@manNomi manNomi left a comment

Choose a reason for hiding this comment

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

매우 매우 가독성이 좋고 너무나도 깔끔하게 작성된 코드임이 느껴집니다
너무 깨끗해서 코드가 아니라 거울을 보는 느낌 ...
한수 배웠습니다 시간나실때 부탁드립니다!

#245

@@ -0,0 +1,104 @@
import { Console } from '@woowacourse/mission-utils';
import {
Copy link

Choose a reason for hiding this comment

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

상수가 분리되어있어서 깔끔합니다


class Calculator {
constructor() {
this.delimiterList = [...DEFAULT_DELIMITERS]; // 구분자
Copy link

Choose a reason for hiding this comment

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

기본 구분자 까지 상수로 관리하니까 확실히 실수를 줄일수 있겠네요 배웠습니다 !!

}

// 커스텀 구분자 파싱
parseCustomDelimiter() {
Copy link

Choose a reason for hiding this comment

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

커스텀 구분자를 파싱하는 방법이 다양하게 존재할것 같은데 정규표현식같은 방법만이 방법이 아닌걸 느꼈습니다!!
특정 API에 대해 공부하고 싶으시다면 정규표현식을 사용하는 방법도 참고하시면 좋을것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

커스텀 구분자가 문자열 앞 최대 1회 주어진다는 전제로 코드 의도를 잘 드러내는 것에 초점을 두고 구현하였습니다!

다음에는 정규표현식을 통해 파싱하는 법도 공부해서 적용해보곘습니다. 좋은 의견 감사합니다~

if (i + 1 === this.size() || this.delimiterList.includes(c)) {
const number = Number(chunk); // 형변환

// 숫자가 아닐 경우
Copy link

Choose a reason for hiding this comment

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

validate로직이 무겁지 않아 같이 써도 괜찮은것 같습니다
추후 확장성을 고려한다면 분리해도 좋겠다는 생각이 듭니다!

Copy link
Author

Choose a reason for hiding this comment

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

해당 메서드 로직이 상대적으로 복잡해서 걱정했었는데 다행이네요.

manNomi님처럼 유효성 검사와 에러 처리 부분을 분리하는 방법이 확장성을 고려했을 때 훨씬 좋을 것 같습니다~

}

// 커스텀 구분자 파싱
parseCustomDelimiter() {
Copy link

Choose a reason for hiding this comment

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

parse함수라서 parsing된 변수가 반환될것 같았는데
끝에서 push해주는 로직이라 함수명을 조금 수정해도 좋을것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

좋은 의견인 것 같습니다 ㅎㅎ

Copy link

@inseong01 inseong01 left a comment

Choose a reason for hiding this comment

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

리드미 기능대로 로직을 구성해서 코드를 파악하기 편했어요!
가독성을 조금 더 보완한다면 calculate 매서드에서 인자를 넘기는 방식으로 코드가 이어지도록 수정하면 좋을 거 같아요.

시간 되실 때 한 번 리뷰해주시면 감사하겠습니다~
#44

Comment on lines +18 to +23
calculate() {
this.parseCustomDelimiter();

const numbers = this.parseNumber();
return this.sum(numbers);
}
Copy link

@inseong01 inseong01 Oct 21, 2025

Choose a reason for hiding this comment

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

Suggested change
calculate() {
this.parseCustomDelimiter();
const numbers = this.parseNumber();
return this.sum(numbers);
}
calculate() {
const parsedInput = this.parseCustomDelimiter();
const numbers = this.parseNumber(parsedInput);
return this.sum(numbers);
}

지금 parseCustomDelimiter 매서드 역할이 기본 문자열을 처리하지 않아서 오류가 날 텐데 이런 형식으로 작성하면 각 매서드마다 역할이 명확하게 보일 거 같아요.

Copy link
Author

@whitecity01 whitecity01 Oct 21, 2025

Choose a reason for hiding this comment

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

먼저 좋은 리뷰 감사합니다! 시간 될 때 꼭 리뷰하러 가겠습니다! ㅎㅎ

parseCustomDelimiter는 this.inputText에 입력된 문자열 중 커스텀 구분자가 있을 경우에만 파싱하도록 의도하였습니다.
혹시 어떤 케이스에서 오류가 발생할 수 있는지 구체적으로 알려주실 수 있을까요?

Copy link
Author

@whitecity01 whitecity01 Oct 21, 2025

Choose a reason for hiding this comment

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

각 메서드의 결과값을 다음 메서드의 인자로 주고 받는 형식이 클래스의 응집도를 낮춘다고 생각하여, 클래스 속성을 공유하는 방식으로 구현하고자 하였습니다.

다만, parseNumber는 조금 더 범용적으로 사용할 수 있을 것 같아 인자 전달 방식으로 구현했는데 이 부분도 통합하는 편이 나을까요?

만약 통합을 하는 것이 좋다면, inseong01님은 어떤 방식이 더 적합하다고 생각하는지 궁금합니다!

Copy link

@inseong01 inseong01 Oct 22, 2025

Choose a reason for hiding this comment

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

parseCustomDelimiter는 this.inputText에 입력된 문자열 중 커스텀 구분자가 있을 경우에만 파싱하도록 의도하였습니다. 혹시 어떤 케이스에서 오류가 발생할 수 있는지 구체적으로 알려주실 수 있을까요?

아, 말씀드린 건 저 코드로 수정하면 parseCustomDelimiter 함수 반환값이 undefined여서 그랬습니다!

Choose a reason for hiding this comment

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

만약 통합을 하는 것이 좋다면, inseong01님은 어떤 방식이 더 적합하다고 생각하는지 궁금합니다!

아하 그런 의도가 있었군요. 공유 방식은 어디서 속성이 변경되었는지 추적하기 어렵고 테스트 준비하는 게 까다로워서 저는 인자를 넘기는 방식으로 작성하는 편이에요!

parseNumber 매서드를 범용적으로 사용할 목적이라면 인자를 받고 값을 반환하는 방향이 코드 흐름을 잘 보여준다고 생각해서 통합하는 방식으로 진행할 거 같아요.

혹시 통합이

const parsedInput = this.parseCustomDelimiter();
const numbers = this.parseNumber(parsedInput);
return this.sum(numbers);

이렇게 작성하는 걸 의미하는 건가요??

Copy link
Author

Choose a reason for hiding this comment

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

네 맞습니다.
현재 parseCustomDelimiter는 클래스 변수를 공유해서 수정하는 방식이고,
parseNumber는 인자를 받고 결괏값을 반환하는 방식이라 두 방식 중 하나로 모두 통합해서 작성하는 건 어떨지 여쭤본 것입니다.

인자를 받고 값을 반환하는 방식

const parsedInput = this.parseCustomDelimiter();
const numbers = this.parseNumber(parsedInput);
return this.sum(numbers);

인자를 받고 값을 반환하는 방식

constructor() {
  this.delimiterList;
  this.inputText;
  this.numbers = []; // 추가
  this.result = 0; // 추가
}

this.parseCustomDelimiter();
this.parseNumber(parsedInput);
this.sum();
return this.result;

이런식으로 나뉠 것 같습니다 ㅎㅎ

Copy link

@JuHyeong424 JuHyeong424 left a comment

Choose a reason for hiding this comment

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

안녕하세요! 상수 분리를 잘 하셨네요!

Choose a reason for hiding this comment

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

구분자랑 input, output, error 문자열을 상수로 두니 관리하기 편해 보입니다.

Copy link
Author

Choose a reason for hiding this comment

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

좋은 리뷰 감사합니다! ㅎㅎ

@@ -0,0 +1,7 @@
export const DEFAULT_DELIMITERS = [',', ':']; // 기본 구분자

Choose a reason for hiding this comment

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

상수로 꼼꼼히 분리하신 게 정말 멋있네요👍

현재는 상수를 전부 각각 export하고 있는데,
import할 메시지가 많아지면 import문이 너무 길어질 수 있을 것 같아요.

같은 뉘앙스의 친구들끼리 하나의 객체로 만들어서 관리해줘도 괜찮을것같다는 생각이 드네요.

예를 들어,

  • 에러 메시지는 한 번 정하면 거의 안 바뀜
  • 출력 문구나 구분자 설정은 상대적으로 자주 바뀜

Errors , Messages, Delimiters 처럼 도메인별 작은 네임스페이스로 나눠도 좋을것같아요.

  • 변경 주기나 책임 분리
  • import문이 깔끔해짐
export const Delimiters = {
  DEFAULT: [',', ':'],
};

export const Messages = {
  INPUT: '덧셈할 문자열을 입력해 주세요.\n',
  OUTPUT_PREFIX: '결과 : ',
};

export const Errors = {
  INVALID_FORMAT: '[ERROR] 입력 형식이 올바르지 않습니다.',
  NON_POSITIVE: '[ERROR] 양수를 입력해주세요.',
};

Copy link
Author

Choose a reason for hiding this comment

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

와 이렇게 하면 굳이 파일로 도메인을 분리할 필요 없이 깔끔하게 분리할 수 있을 것 같습니다
좋은 피드백 감사합니다~!

@@ -0,0 +1,104 @@
import { Console } from '@woowacourse/mission-utils';

Choose a reason for hiding this comment

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

let은 대부분 split/map 조합으로 정리 가능해보여요.
예를 들어 chunk 누적 대신

this.inputText.split(delimiter).map(Number);

이런 식으로 처리하면 루프 안에서 상태 바꿀 필요도 없고 전부 const로 바꿀 수 있을 듯합니다.
size()도 매번 계산하기보다 미리 변수에 담아두면 불필요한 let i 루프도 정리될 것 같네요!

Copy link
Author

Choose a reason for hiding this comment

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

엇 split은 하나의 인자만 받을 수 있지 않나요?
구분자가 최대 3개까지 올 수 있다고 생각해서 해당 방식으로는 더 복잡해질 수 있다고 판단하였었습니다!

size가 매번 계산되는 건 생각하지 못했던 부분인데 좋은 의견인 것 같습니다! 감사합니다 ㅎㅎ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants