-
Notifications
You must be signed in to change notification settings - Fork 232
[문자열 덧셈 계산기] 김승제 미션 제출합니다. #65
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
base: main
Are you sure you want to change the base?
Changes from all commits
44c1727
44f3ea7
c5d814c
3d7c807
01b25b7
f8d550d
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,13 @@ | ||
| # javascript-calculator-precourse | ||
| # javascript-calculator-precourse | ||
|
|
||
| # 기능 목록 | ||
|
|
||
| 1. **입력 및 출력** 기능 | ||
|
|
||
| 2. **커스텀 구분자 파싱** 기능 | ||
|
|
||
| 3. 구분자(+커스텀)로 문자열의 **숫자 분리** 기능 | ||
|
|
||
| 4. 3번의 유효성 검증 실패 시 **에러 핸들링** 기능 | ||
|
|
||
| 5. 3번의 유효성 검증 성공 시 구분자로 분리한 **각 숫자의 합**을 구하는 기능 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,14 @@ | ||
| import Calculator from './calculator.js'; | ||
|
|
||
| class App { | ||
| async run() {} | ||
| async run() { | ||
| const calculator = new Calculator(); | ||
|
|
||
| await calculator.input(); // 입력 | ||
| const result = calculator.calculate(); | ||
|
|
||
| calculator.output(result); // 출력 | ||
| } | ||
| } | ||
|
|
||
| export default App; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,104 @@ | ||||||||||||||||||||||||
| import { Console } from '@woowacourse/mission-utils'; | ||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||
|
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. 상수가 분리되어있어서 깔끔합니다 |
||||||||||||||||||||||||
| DEFAULT_DELIMITERS, | ||||||||||||||||||||||||
| ERROR_INVALID_FORMAT, | ||||||||||||||||||||||||
| ERROR_NON_POSITIVE, | ||||||||||||||||||||||||
| INPUT_MESSAGE, | ||||||||||||||||||||||||
| OUTPUT_PREFIX, | ||||||||||||||||||||||||
| } from './constants.js'; | ||||||||||||||||||||||||
| import { isNumberChar } from './utils.js'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| class Calculator { | ||||||||||||||||||||||||
| constructor() { | ||||||||||||||||||||||||
| this.delimiterList = [...DEFAULT_DELIMITERS]; // 구분자 | ||||||||||||||||||||||||
|
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. 기본 구분자 까지 상수로 관리하니까 확실히 실수를 줄일수 있겠네요 배웠습니다 !! |
||||||||||||||||||||||||
| this.inputText = ''; // 입력 문자열 | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 문자열 연산 | ||||||||||||||||||||||||
| calculate() { | ||||||||||||||||||||||||
| this.parseCustomDelimiter(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const numbers = this.parseNumber(); | ||||||||||||||||||||||||
| return this.sum(numbers); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+18
to
+23
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.
Suggested change
지금
Author
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. 먼저 좋은 리뷰 감사합니다! 시간 될 때 꼭 리뷰하러 가겠습니다! ㅎㅎ parseCustomDelimiter는 this.inputText에 입력된 문자열 중 커스텀 구분자가 있을 경우에만 파싱하도록 의도하였습니다.
Author
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. 각 메서드의 결과값을 다음 메서드의 인자로 주고 받는 형식이 클래스의 응집도를 낮춘다고 생각하여, 클래스 속성을 공유하는 방식으로 구현하고자 하였습니다. 다만, parseNumber는 조금 더 범용적으로 사용할 수 있을 것 같아 인자 전달 방식으로 구현했는데 이 부분도 통합하는 편이 나을까요? 만약 통합을 하는 것이 좋다면, inseong01님은 어떤 방식이 더 적합하다고 생각하는지 궁금합니다! 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.
아하 그런 의도가 있었군요. 공유 방식은 어디서 속성이 변경되었는지 추적하기 어렵고 테스트 준비하는 게 까다로워서 저는 인자를 넘기는 방식으로 작성하는 편이에요!
혹시 통합이 이렇게 작성하는 걸 의미하는 건가요??
Author
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. 네 맞습니다. 인자를 받고 값을 반환하는 방식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;이런식으로 나뉠 것 같습니다 ㅎㅎ |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 배열의 숫자들을 더한 후 반환 | ||||||||||||||||||||||||
| sum(numbers) { | ||||||||||||||||||||||||
| return numbers.reduce((acc, cur) => acc + cur, 0); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 문자열 길이 반환 | ||||||||||||||||||||||||
| size() { | ||||||||||||||||||||||||
| return this.inputText.length; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 커스텀 구분자 파싱 | ||||||||||||||||||||||||
| parseCustomDelimiter() { | ||||||||||||||||||||||||
|
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. 커스텀 구분자를 파싱하는 방법이 다양하게 존재할것 같은데 정규표현식같은 방법만이 방법이 아닌걸 느꼈습니다!!
Author
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회 주어진다는 전제로 코드 의도를 잘 드러내는 것에 초점을 두고 구현하였습니다! 다음에는 정규표현식을 통해 파싱하는 법도 공부해서 적용해보곘습니다. 좋은 의견 감사합니다~ 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. parse함수라서 parsing된 변수가 반환될것 같았는데
Author
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 (this.size() < 5) return; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 구분자가 없거나 틀린 형식일 시 return | ||||||||||||||||||||||||
| if (this.inputText[0] !== '/') return; | ||||||||||||||||||||||||
| if (this.inputText[1] !== '/') return; | ||||||||||||||||||||||||
| if (this.inputText[3] !== '\\') return; | ||||||||||||||||||||||||
| if (this.inputText[4] !== 'n') return; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| this.delimiterList.push(this.inputText[2]); // 커스텀 구분자 추가 | ||||||||||||||||||||||||
| this.inputText = this.inputText.substring(5); // 커스텀 구분자 문자열 삭제 | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // delimiterList(구분자)로 문자열의 숫자를 파싱해서 배열로 반환 | ||||||||||||||||||||||||
| // 예외 발생 시 애플리케이션이 종료된다. | ||||||||||||||||||||||||
| parseNumber() { | ||||||||||||||||||||||||
| const numbers = [0]; // 기본값 : 0 | ||||||||||||||||||||||||
| let chunk = ''; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| for (let i = 0; i < this.size(); i++) { | ||||||||||||||||||||||||
| const c = this.inputText[i]; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // chunk에 숫자 추가 | ||||||||||||||||||||||||
| if (isNumberChar(c)) { | ||||||||||||||||||||||||
| chunk += c; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (i + 1 < this.size()) continue; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 문자열의 마지막이나 구분자를 만날 경우 | ||||||||||||||||||||||||
| if (i + 1 === this.size() || this.delimiterList.includes(c)) { | ||||||||||||||||||||||||
| const number = Number(chunk); // 형변환 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 숫자가 아닐 경우 | ||||||||||||||||||||||||
|
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. validate로직이 무겁지 않아 같이 써도 괜찮은것 같습니다
Author
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. 해당 메서드 로직이 상대적으로 복잡해서 걱정했었는데 다행이네요. manNomi님처럼 유효성 검사와 에러 처리 부분을 분리하는 방법이 확장성을 고려했을 때 훨씬 좋을 것 같습니다~ |
||||||||||||||||||||||||
| if (Number.isNaN(number)) throw new Error(ERROR_INVALID_FORMAT); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 양수가 아닐 경우 | ||||||||||||||||||||||||
| if (number <= 0) throw new Error(ERROR_NON_POSITIVE); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| numbers.push(number); | ||||||||||||||||||||||||
| chunk = ''; | ||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 숫자, 구분자 모두 아닐 경우 | ||||||||||||||||||||||||
| throw new Error(ERROR_INVALID_FORMAT); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return numbers; // 성공 시 분리된 숫자 배열을 반환 | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 문자열 입력 | ||||||||||||||||||||||||
| async input() { | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| const input = await Console.readLineAsync(INPUT_MESSAGE); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| this.inputText = input; | ||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||
| throw new Error(ERROR_INVALID_FORMAT); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // 결과 출력 | ||||||||||||||||||||||||
| output(result) { | ||||||||||||||||||||||||
| Console.print(`${OUTPUT_PREFIX}${result}`); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export default Calculator; | ||||||||||||||||||||||||
|
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. 구분자랑 input, output, error 문자열을 상수로 두니 관리하기 편해 보입니다.
Author
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,7 @@ | ||
| export const DEFAULT_DELIMITERS = [',', ':']; // 기본 구분자 | ||
|
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. 상수로 꼼꼼히 분리하신 게 정말 멋있네요👍 현재는 상수를 전부 각각 export하고 있는데, 같은 뉘앙스의 친구들끼리 하나의 객체로 만들어서 관리해줘도 괜찮을것같다는 생각이 드네요. 예를 들어,
export const Delimiters = {
DEFAULT: [',', ':'],
};
export const Messages = {
INPUT: '덧셈할 문자열을 입력해 주세요.\n',
OUTPUT_PREFIX: '결과 : ',
};
export const Errors = {
INVALID_FORMAT: '[ERROR] 입력 형식이 올바르지 않습니다.',
NON_POSITIVE: '[ERROR] 양수를 입력해주세요.',
};
Author
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. 와 이렇게 하면 굳이 파일로 도메인을 분리할 필요 없이 깔끔하게 분리할 수 있을 것 같습니다 |
||
|
|
||
| export const INPUT_MESSAGE = '덧셈할 문자열을 입력해 주세요.\n'; | ||
| export const OUTPUT_PREFIX = '결과 : '; | ||
|
|
||
| export const ERROR_INVALID_FORMAT = '[ERROR] 입력 형식이 올바르지 않습니다.'; | ||
| export const ERROR_NON_POSITIVE = '[ERROR] 양수를 입력해주세요.'; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export const isNumberChar = (c) => { | ||
| return '0' <= c && c <= '9'; | ||
| }; |
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.
let은 대부분 split/map 조합으로 정리 가능해보여요.예를 들어
chunk누적 대신이런 식으로 처리하면 루프 안에서 상태 바꿀 필요도 없고 전부
const로 바꿀 수 있을 듯합니다.size()도 매번 계산하기보다 미리 변수에 담아두면 불필요한let i루프도 정리될 것 같네요!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.
엇 split은 하나의 인자만 받을 수 있지 않나요?
구분자가 최대 3개까지 올 수 있다고 생각해서 해당 방식으로는 더 복잡해질 수 있다고 판단하였었습니다!
size가 매번 계산되는 건 생각하지 못했던 부분인데 좋은 의견인 것 같습니다! 감사합니다 ㅎㅎ