Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
0b02115
docs : 기능요구사항을 정의하다
manNomi Oct 14, 2025
100e4f2
docs: 클래스를 문서화하다
manNomi Oct 15, 2025
0811d15
feat: 프로젝트 구조를 구현하다
manNomi Oct 15, 2025
affdc13
test: 넘버클래스 테스트코드를 작성하다
manNomi Oct 15, 2025
7e295b1
feat: number 클래스의 add함수를 구현하다
manNomi Oct 15, 2025
66407c5
test: parse 테스트코드를 작성하다
manNomi Oct 15, 2025
bafbdb0
feat: parse 함수를 구현하다
manNomi Oct 15, 2025
dfa88c5
test: parser usecase를 작성하다
manNomi Oct 15, 2025
48a1ab8
test: 백이스케이프를 수정하다
manNomi Oct 15, 2025
9894e9a
test: Extraction test코드를 작성하다
manNomi Oct 15, 2025
f1d73e6
test: test코드의 함수에 input 매개변수를 추가하다
manNomi Oct 15, 2025
0d7919d
feat: 커스텀 구분자 추출하기를 구현하다
manNomi Oct 15, 2025
b3aba35
feat: input,output View를 구현하다
manNomi Oct 16, 2025
edeb954
feat: contooler 클래스 초기코드를 구현하다
manNomi Oct 16, 2025
8675344
test: controller의 테스트코드를 작성하다
manNomi Oct 16, 2025
953a6b1
feat: contooler 클래스의 run함수를 구현하다
manNomi Oct 16, 2025
13d5e5f
test: controller 클래스 테스트 모킹 함수를 수정하다
manNomi Oct 16, 2025
3c0be91
feat: contooler 클래스의 run 함수를 연결하다
manNomi Oct 16, 2025
d7c9eda
test: validate에 대한 유즈케이스를 정의하다
manNomi Oct 16, 2025
c69a58b
test: validate에 대한 테스트 케이스를 작성하다
manNomi Oct 16, 2025
5e7598b
docs: 음수 입력예외사항을 추가하다
manNomi Oct 16, 2025
979cd65
test: 음수 미포함하도록 함수를 수정하다
manNomi Oct 16, 2025
3e7b889
fix: 음수를 예외처리하도록 수정하다
manNomi Oct 16, 2025
ce20c3f
feat: isLong함수를 구현하다
manNomi Oct 16, 2025
8286f1e
fix: 함수 테스트를 위해 테스트코드를 수정하다
manNomi Oct 16, 2025
a23f88b
fix: isIntegers 함수를 수정하다
manNomi Oct 16, 2025
123ea27
feat: validate를 구현하다
manNomi Oct 17, 2025
5691a8a
test: validate 테스트코드에서 숫자가 string으로 감싸진 오타를 수정하다
manNomi Oct 17, 2025
f4c7145
fix: 음수 입력에 대한 예외처리 로직을 구현하다
manNomi Oct 17, 2025
0434003
test: 커스텀 구분자에 대한 테스트를 추가정의하다
manNomi Oct 17, 2025
70bd6d4
test: 디버깅을 위해 추출테스트를 진행하다
manNomi Oct 17, 2025
cd1b854
test: 공백을 파싱하는 테스트코드를 작성하다
manNomi Oct 17, 2025
24d2557
fix: 공백을 허용하도록 수정하다
manNomi Oct 17, 2025
cfa41e4
fix: 커스텀 구분자를 여러개 처리하도록 수정하다
manNomi Oct 17, 2025
2f8b73d
fix: parser가 커스텀구분자를 추출하지 못하던 버그를 수정하다
manNomi Oct 17, 2025
7725021
fix: isLong 함수를 제거하다
manNomi Oct 18, 2025
3ebf241
test: 인덱스 범위를 넘어선 값을 반환하는 버그를 수정
manNomi Oct 18, 2025
a40d618
docs: 구현된 요구사항 반영하다
manNomi Oct 18, 2025
796265e
test: 커스텀 구분자가 여러개인 경우 테스트 코드작성
manNomi Oct 18, 2025
c23a799
fix: 커스텀 구분자가 여러개일때 파싱안되던 버그를 수정하다
manNomi Oct 18, 2025
adfa885
fix: 커스텀구분자 여러개 사이에 숫자가 파싱안되던 버그를 수정
manNomi Oct 18, 2025
d25d18e
test: 커스텀 구분자에 애러 추적툴을 구현하다
manNomi Oct 18, 2025
636a1a0
fix: Parser에 정규식이 포함되는 경우를 수정하다
manNomi Oct 18, 2025
42b88ca
test: 랜덤 문자열 생성기에서 숫자를 커스텀구분자로 포함하지 않도록 수정하다
manNomi Oct 18, 2025
ed29065
test: 랜덤 문자열 생성기에 커스텀 구분자 추출테스트를 구현하다
manNomi Oct 18, 2025
15fc699
docs: 추가사항을 작성하다
manNomi Oct 19, 2025
1a9a935
fix: 공백을 미허용 하도록 수정하다
manNomi Oct 19, 2025
15179a6
refactor: 상수를 분리하다
manNomi Oct 19, 2025
c1ee356
fix: isNumber로 함수명을 수정하다
manNomi Oct 19, 2025
81af4f4
refactor: parse함수 테스트 위해분리하다
manNomi Oct 19, 2025
d4f6f13
test: 기대값과 실제값을 반대로 작성한 테스트코드를 수정하다
manNomi Oct 19, 2025
94aa076
docs: 추가사항을 작성하다
manNomi Oct 19, 2025
77b99ac
fix: parsing이 제대로 되지않던 버그를 수정하다
manNomi Oct 19, 2025
9e67442
test: 전반적인 테스트코드 개선하다
manNomi Oct 19, 2025
88f66bf
refactor: parser가 regex를 관리하도록 위임하다
manNomi Oct 19, 2025
91dbfac
refactor: extractor의 반환값을 수정하다
manNomi Oct 19, 2025
52d4557
fix: /나 \ 가 포함된경우 연속된 구분자 처리를 못하던 버그를 수정하다
manNomi Oct 19, 2025
df765a8
test: 테스트를 반복하도록 코드를 수정 리팩토링하다
manNomi Oct 19, 2025
7d28a80
refactor: validate 함수 리팩토링하다
manNomi Oct 19, 2025
03c0267
refactor: regex관련 요소를 분리하다
manNomi Oct 19, 2025
b66422a
refactor: Parser 관련 코드를 정리하다
manNomi Oct 19, 2025
3f482f9
refactor: Extraction 관련 코드를 리팩토링하다
manNomi Oct 19, 2025
30972ee
style: 주석을 적다
manNomi Oct 19, 2025
11a2cee
feat: 소수 검증을 추가하다
manNomi Oct 19, 2025
6c44eac
feat: 구분자가 한글자만 허용할것인지 수정하다
manNomi Oct 19, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,57 @@
# javascript-calculator-precourse
# javascript-calculator-precourse

# 문자열 덧셈 계산기 개발

## 1. 기능요구 사항

-[X] 커스텀 구분자를 추출하는 기능 -[X] 구분자를 통해 입력받은 숫자를 파싱하는 기능 -[X] 파싱된 입력이 올바른지 확인하는 기능 -[X] 파싱된 입력을 더하는 기능

- Model
- Parser : 구분자
- Number : 입력받은 숫자
- Extraction : 추출자
- RandomMaker : 랜덤 제작기

- View
- InputView : 입력
- OutputView : 출력

- Controller
- Calculator : 계산기

### - 입력 요구사항

-[X] 구분자와 야수로 구성된 문자열 -[X] 구분자 : `,` `:` `//커스텀구분자\n`

#### - 추가사항

-[X] 음수 입력시 ERROR 처리

### - 출력 요구사항

-[X] "결과 : {int}" -[X] 잘못된 입력시 [ERROR]로 시작하는 메시지와 함께 종료

## 2. 프로그래밍 요구 사항

-[X] 프로그래밍의 시작지점은 App.js의 run() 인가? -[X] package.json 파일을 변경하지 않았는가? -[X] 프로그램 종료시 process.exit()를 호출하지 않았는가? -[X] 자바스크립트 코드 컨벤션에 맞게 작성했는가? -[X] @woowacourse/mission-utils에서 제공하는 Console API를 사용해서 구현했는가?

## 3. 도전 사항

Choose a reason for hiding this comment

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

도전사항 세세하게 작성하신거 진짜 대단하십니다...

-[X] TDD 설계원칙 적용 -[X] 테스트 코드 랜덤 문자열 생성기 구현 -[X] MVC 패턴 적용하기 -[X] 객체지향 원칙 준수하기 -[X] AI 사용 없이 구현하기

Choose a reason for hiding this comment

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

AI 사용 없이 구현에 성공하셨군요..!! 대단하십니다 ..👍🏻


## 4. 예상 실행 결과

실행 결과 예시
덧셈할 문자열을 입력해 주세요.
1,2:3
결과 : 6

### 5. 추가사항

- 연속된 구분자의 입력을 허용할것인가 - 미허용
- 공백 입력을 허용할것인가 - 미허용
- 소수점을 허용할것인가 - 미허용
-> 소수점이 포함된경우 커스텀 구분자로 . 이 오면 허용할것인가
- 커스텀 구분자 여러개를 허용할것 인가 ? - 허용
- 커스텀 구분자가 중간에 나오는것을 허용할것인가? - 미 허용
- 커스텀 구분자가 . 일때 .. 처리를 어케할것인지 - 미 허용 구분자는 한글자
20 changes: 10 additions & 10 deletions __tests__/ApplicationTest.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import App from "../src/App.js";
import { MissionUtils } from "@woowacourse/mission-utils";
import { MissionUtils } from '@woowacourse/mission-utils';
import App from '../src/App.js';

const mockQuestions = (inputs) => {
MissionUtils.Console.readLineAsync = jest.fn();
Expand All @@ -11,18 +11,18 @@ const mockQuestions = (inputs) => {
};

const getLogSpy = () => {
const logSpy = jest.spyOn(MissionUtils.Console, "print");
const logSpy = jest.spyOn(MissionUtils.Console, 'print');
logSpy.mockClear();
return logSpy;
};

describe("문자열 계산기", () => {
test("커스텀 구분자 사용", async () => {
const inputs = ["//;\\n1"];
describe('문자열 계산기', () => {
test('커스텀 구분자 사용', async () => {
const inputs = ['//;\\n1'];
mockQuestions(inputs);

const logSpy = getLogSpy();
const outputs = ["결과 : 1"];
const outputs = ['결과 : 1'];

const app = new App();
await app.run();
Expand All @@ -32,12 +32,12 @@ describe("문자열 계산기", () => {
});
});

test("예외 테스트", async () => {
const inputs = ["-1,2,3"];
test('예외 테스트', async () => {
const inputs = ['-1,2,3'];
mockQuestions(inputs);

const app = new App();

await expect(app.run()).rejects.toThrow("[ERROR]");
await expect(app.run()).rejects.toThrow('[ERROR]');
});
});
7 changes: 6 additions & 1 deletion src/App.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import Controller from './controller/Controller.js';

class App {
async run() {}
async run() {
const controller = new Controller();
await controller.run();
}
}

export default App;
11 changes: 11 additions & 0 deletions src/constant/error.js
Copy link

Choose a reason for hiding this comment

The 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,11 @@
export const ERROR_MESSAGE = {

Choose a reason for hiding this comment

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

런타임에서 변하지 않을 에러 메시지라면 Object.freeze를 사용하는 것도 좋다고 생각합니다.
혹시 일부러 사용하시지 않은거라면, 어떤 이유였을지 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

평소에 개발을 할때에는 object.freez가 필요가 없었어서 자주 사용을 안하다보니 안좋은 습관이 들은것 같습니다..
말씀하신것처럼 반영하는게 더 좋을것이라고 판단이 되네요

이번기회에 object.freez에 대해서 조금더 알게되었는데 얕은불변성만 보장한다는 사실을 처음 알게되었습니다 감사합니다!!

NOT_MINUS: '[ERROR] 음수가 포함되어 있습니다.',
NOT_EMPTY: '[ERROR] 공백이 포함되어 있습니다.',
NOT_NUMBER: '[ERROR] 숫자가 아닌것이 포함되어 있습니다.',
DELIMITER_CONTINUOUS: '[ERROR] 연속된 구분자가 존재합니다.',
DELIMITER_EMPTY: '[ERROR] 구분자가 공백입니다.',
DELIMITER_HAS_WHITESPACE: '[ERROR] 구분자에 공백이 포함되어 있습니다.',
DELIMITER_HAS_NUMBER: '[ERROR] 구분자에 숫자가 포함되어 있습니다.',
DELIMITER_NOT_SINGLE_CHAR: '[ERROR] 구분자는 한 글자여야 합니다.',
NOT_INTEGER: '[ERROR] 소수가 포함되어 있습니다.',
Comment on lines +1 to +10
Copy link

Choose a reason for hiding this comment

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

NOT_MINUS나 NOT_NUMBER에 어떤 게 음수가 아니고 어떤 게 숫자가 아닌지 불분명한 것 같아서
처음 보는 사람이 코드를 읽으면 정확히 눈에 안 들어올 수도 있을 것 같습니다!

에러 메시지는 어차피 상수이니 조금 더 구체적으로 명시가 되면 가독성이 더 좋아질 것 같아요 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

NOT_MINUS인데 막상 글자와 하는역할은 포함되어 있다고 느껴지네요 말씀감사합니다

};
Comment on lines +1 to +11

Choose a reason for hiding this comment

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

이런식으로 객체로 묶어서 constants를 표현하는 것도 좋은 것 같아요!
좋은 방식 배워갑니다:)

25 changes: 25 additions & 0 deletions src/constant/regex.js

Choose a reason for hiding this comment

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

문자열이나 상수를 constant로 묶은 것은 많이 봤는데, regex 관련된 것들만 따로 모아둔 것이 신선한거 같아요.
다른 분들 코드를 봤을 때는 util과 관련된 것 끼리 묶었는데, util 말고 regex와 관련된 것을 묶으신 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

regex를 따로 모아두고 util 대신 constant로 쓴 이유를 여쭤보신것 맞나요 ?
regex 또한 변하지않는 상수로 판이되어서 util 보다는 상수로 관리했고 !!
간단한 함수들을 util에 또 분리한다면 가독성을 해칠것 같아 같이 두었는데

고민이 되는게 지금은
regex 함수가 초점이면 utils regex 보관이 초점이면 constant에 보관할것 같은데 생각해보게 하는 좋은 글이네요 감사합니다

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
export const REGEX_PATTERNS = {
CUSTOM_DELIMITER_EXTRACT: /\/\/(.*?)\\n/g,
CUSTOM_DELIMITER_DEFINITION: /\/\/.+?\\n/g,
Comment on lines +1 to +3

Choose a reason for hiding this comment

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

이 코드라면 \ + n 를 찾아서 \n 처리에 문제가 생길 수도 있지않을까 궁금합니다!

REGEX_SPECIAL_CHARS: /[.*+?^${}()|[\]\\]/g,
WHITESPACE: /\s/,
DIGIT: /\d/,
};

Copy link

Choose a reason for hiding this comment

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

정규식을 한번에 묶어둘 생각은 못했는데, 정규식이 코드에서 반복되는 것을 막아 좋은 것 같네요!

export const RegexUtils = {
Copy link

Choose a reason for hiding this comment

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

간단한 함수들이라서 constant 안에 두신 것 같은데 이렇게 두는 것도 괜찮아 보이네요

escapeRegexChars(str) {
return str.replace(REGEX_PATTERNS.REGEX_SPECIAL_CHARS, '\\$&');
},

createContinuousPattern(delimiter) {
return new RegExp(`${delimiter}{2,}`);

Choose a reason for hiding this comment

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

문자열의 패턴을 관리하는 RegExp객체가 있다는 것을 새롭게 알게 되었습니다.
반복적으로 사용하는 정규식을 한 곳에 모아두니 코드가 훨씬 깔끔해지네요! 😊

},

createSplitPattern(delimiters) {
return new RegExp(delimiters.join('|'));
},

createCusomPattern(delimiter) {
return `//${delimiter}\\n`;
},
};
40 changes: 40 additions & 0 deletions src/controller/Controller.js

Choose a reason for hiding this comment

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

다른 분들 코드를 참고해보니까 Extraction, Number, Parser를 Calculator class에서 다루시는 분들이 많더라고요! 개인적으로 Calculator에서 관리하는것도 괜찮아 보입니다!!

Copy link
Author

Choose a reason for hiding this comment

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

말씀주신 내용 전적으로 공감합니다
저 또한 너무 많은 분리가 오히려 가독성을 해치는것 같기도 하고
class 가 가지는 함수가 하나 뿐이라서 합치는게 오히려 올바르다고 생각이 드네요
행위 자체를 객체로 설정한것도 후회가 되고요 말씀 감사합니다!!

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import Extraction from '../service/model/Extraction.js';
import Parser from '../service/model/Parser.js';
import inputView from '../view/InputView.js';
import outputView from '../view/OutputView.js';
import Number from '../service/model/Number.js';
import validate from '../service/validate/validate.js';

export default class Controller {
constructor() {
this.extraction = new Extraction();
this.parser = new Parser();
}

async run() {
try {
const input =
await inputView.readLineMessage('덧셈할 문자열을 입력해 주세요.');
Comment on lines +16 to +17

Choose a reason for hiding this comment

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

에러메세지에 대해 상수로 분리하신 것을 보았습니다.
통일성을 위해 이 입력 문자열도 상수화 하는 것이 좋아보이는데 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

지적 해주셔서 알게되었ㄴ네요...
상수화 한다고 해놓고 까먹었네요 실제 프로젝트다면 꼼꼼하게 봐주신 작성자 분덕분에
실수 하나를 줄일 수 있었겠네요 감사합니다!!


// 구분자 추출 {raw:원본,escaped:정규식 이스케이프}
const { raw, escaped } = this.extraction.extractCustomDelimiters(input);
validate.validateDelimiters(raw);
Copy link

Choose a reason for hiding this comment

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

예외 처리를 따로 빼서 확실히 분리하신 것이 좋아보입니다!


const textWithoutDefinitions =
this.parser.removeCustomDelimiterDefinitions(input, raw);
validate.validateContinuousDelimiters(textWithoutDefinitions, escaped);
Comment on lines +23 to +25

Choose a reason for hiding this comment

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

변수명이 자세해서 특별한 주석 없이도 가독성이 좋은 것 같아요! 👀


const numbers = this.parser.parseToNumbers(
textWithoutDefinitions,
escaped,
);
validate.validateNumbers(numbers);

const numberModel = new Number(numbers);
await outputView.printMessage(`결과 : ${numberModel.getAddedNumbers()}`);
} catch (error) {
await outputView.printMessage(error.message);
throw new Error(error.message);
}
}
}
20 changes: 20 additions & 0 deletions src/service/model/Extraction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { REGEX_PATTERNS, RegexUtils } from '../../constant/regex.js';

export default class Extraction {
extractCustomDelimiters(text) {
const delimiters = Array.from(
text.matchAll(REGEX_PATTERNS.CUSTOM_DELIMITER_EXTRACT),
Copy link

@janghw0126 janghw0126 Oct 21, 2025

Choose a reason for hiding this comment

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

matchAll과 Array.from을 함께 사용하는 이유를 명확히 알고 싶습니다!
단순히 match()로는 안 되는 건가요?

Copy link
Author

Choose a reason for hiding this comment

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

저도 처음에는 match를 사용했었는데
커스텀 구분자를 여러글자 받기위해서 matchAll 을 사용했습니다!!
한글자를 입력받게 하신분들이 많더라고요 !!
그래서 아마 차이가 나는것 같습니다

(match) => match[1],
);
return {
raw: delimiters,
escaped: this.#escapeDelimiters(delimiters),
};
}

#escapeDelimiters(delimiters) {
return delimiters.map((delimiter) =>
RegexUtils.escapeRegexChars(delimiter),
);
}
}
11 changes: 11 additions & 0 deletions src/service/model/Number.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export default class Number {

Choose a reason for hiding this comment

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

리뷰하다가 validate.js에서 js 내장 객체인 Number()를 사용하시는데
이 부분 클래스 네이밍이 기존 js의 내장 Number랑 겹칠 수도 있을 것 같아서요!
좀 더 클래스명을 구체적으로 작성하는 것도 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

저도 사용하다 보니 계속 Number가 겹치더라고요
내장코드랑 동일한 함수 , 클래스명을 짓다니 ....
덕분에 다음에는 실수를 줄일 수 있겠네요 감사합니다!

#numbers;

constructor(numbers) {
this.#numbers = numbers;
}

getAddedNumbers() {
return this.#numbers.reduce((acc, cur) => acc + cur);
}
Comment on lines +8 to +10

Choose a reason for hiding this comment

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

빈 문자열 입력 등으로 숫자 배열이 비어질 수 있는데
현재 reduce 함수에는 초기값이 없어서 예외가 발생할 수 있지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

헉 그러네요!! 초기값 지정을 했어야 했는데 감사합니다 !

}
Comment on lines +1 to +11

Choose a reason for hiding this comment

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

Number는 자바스크립트의 내장 객체 이름이라 혼동을 줄 수 있을 것 같습니다.

클래스명을 변경하거나, 단순한 기능이라면 별도 함수로 분리하는 것도 좋을 것 같습니다!

27 changes: 27 additions & 0 deletions src/service/model/Parser.js
Copy link

Choose a reason for hiding this comment

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

전체적으로 함수와 class를 굉장히 잘 활용하신다고 느꼈습니다. 저도 다음엔 기능 별로 나눠보도록 해봐야 겠어요. 많이 배우고 갑니다!

Copy link
Author

Choose a reason for hiding this comment

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

이해하고 쓰는거랑 알고 쓰는것의 차이를 많이 느끼고 있습니다..
그저 method만 알고 방법만 알고 적고 있는 기분이라 다른 팀원들도
class를 적극적으로 활용해서 같이 공부해보면 좋을것 같습니다!!

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { RegexUtils } from '../../constant/regex.js';

const DEFAULT_DELIMITERS = [',', ':'];

export default class Parser {
parseToNumbers(text, escapedDelimiters) {
const allDelimiters = [...escapedDelimiters, ...DEFAULT_DELIMITERS];
const splitPattern = RegexUtils.createSplitPattern(allDelimiters);
return text
.split(splitPattern)
.filter((value) => value !== '')
.map((value) => Number(value));
}

Choose a reason for hiding this comment

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

메서드 체이닝 때문에 코드가 길어지면 가독성이 떨어질 수 있을 것 같아요!
(공유받은 클린코드 공유 자료입니다! 시간 나실때 한번 읽어보세요!!)
https://github.com/woowacourse/woowacourse-docs/blob/main/cleancode/pr_checklist.md

Copy link
Author

Choose a reason for hiding this comment

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

메소드 체이닝 때문에 코드가 읽히기 ㅅ쉽지 않다는것을 저도 느끼고 있습니다..

과도한 분리 어떨때는 너무 심한 체이닝 저 자신을 돌아보게 되네요 좋은 글 공유해주셔서 감사합니다 !

removeCustomDelimiterDefinitions(text, delimiters) {
let result = text;

delimiters.forEach((delimiter) => {
const definition = RegexUtils.createCusomPattern(delimiter);

Choose a reason for hiding this comment

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

여기 부분 진짜 사소하긴 한데 createCustomPattern() 함수 오타 있는 것 같아서 리뷰 남깁니다..!

Copy link
Author

Choose a reason for hiding this comment

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

자바스크립트라 오타에 치명적일텐데!!!
복사붙여넣기를 잘해서 아직 살아있나 봅니다 꼼꼼히 봐주셔 감사합니다 !!

if (result.startsWith(definition)) {
result = result.slice(definition.length);
}
});

return result;
}
}
69 changes: 69 additions & 0 deletions src/service/validate/validate.js

Choose a reason for hiding this comment

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

예외처리 함수가 많지 않기 때문에, 객체로 묶지 않고 각각 export 하는것도 괜찮아 보입니다!

Copy link
Author

Choose a reason for hiding this comment

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

너무 타당한 말씀이네요
각각 export 하면서 선언적으로 사용하면 더더욱 좋다는 생각이 드는것 같습니다

Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { ERROR_MESSAGE } from '../../constant/error.js';
import { REGEX_PATTERNS, RegexUtils } from '../../constant/regex.js';

const DEFAULT_DELIMITERS = [',', ':'];

Choose a reason for hiding this comment

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

Parser.js의 3번째 줄도 같은 코드가 있는 것 같은데 constant 파일로 옮겨 재사용해도 괜찮을 것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

끝나고 코드리뷰를 받다보니 이런 실수들을 잡아주셔서 너무 좋네요 꼼꼼히 봐주셔 감사합니다


const validate = {
Copy link

Choose a reason for hiding this comment

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

제가 모르는 것이 많아서 궁금한데 혹시 validate는 class로 만들지 않으신 이유가 있으신가요?

Copy link
Author

Choose a reason for hiding this comment

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

조금 정곡을 찔린 것 같습니다.

저는 **“행위는 객체로 두지 않는다”**는 원칙을 가지고 있었지만, 실제로는 Parser, Extractor처럼 행위적인 로직도 객체로 만들었던 부분이 있어서 아쉬움이 남는 1주차였습니다.

일관성을 지키려면 validate 역시 클래스가 되어야 맞겠지요.
하지만 validate를 클래스로 두지 않았던 이유는 명확했습니다.
검증에는 두 가지 종류가 있다고 생각했기 때문입니다.
1. 도메인 검증 – 입력이 의도에 맞는지 확인하는 것
2. 문법적 검증 – 입력이 형식적으로 올바른지 확인하는 것

이 두 검증이 클래스 안팎에서 모두 사용될 수 있다면,
validate를 클래스로 두었을 때 오히려 책임이 모호해질 수 있다고 판단했습니다.

앞으로는 문법적 검증은 함수로, 도메인 검증은 클래스 내부 메서드로 분리하여 작성하려 합니다.
좋은 피드백을 통해 제 원칙을 다시 점검할 수 있었던 의미 있는 경험이었습니다.

validateNumbers(numbers) {
numbers.forEach((number) => {
// 공백 체크
if (typeof number === 'string' && number.trim() === '') {
throw new Error(ERROR_MESSAGE.NOT_EMPTY);
}
// 숫자 여부 체크
if (Number.isNaN(Number(number))) {
throw new Error(ERROR_MESSAGE.NOT_NUMBER);
}
// 음수 체크
if (number < 0) {
throw new Error(ERROR_MESSAGE.NOT_MINUS);
}
// 소수 체크
if (!Number.isInteger(Number(number))) {
throw new Error(ERROR_MESSAGE.NOT_INTEGER);
}
});
Comment on lines +7 to +25

Choose a reason for hiding this comment

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

여기서 Number(number)를 여러 번 호출하는 것 같습니다!
number라는 하나의 변수에 대해서 Number()로 변환하는 것이니 상단에

const num = Number(number)

로 변환은 한번만 수행하고, 비교 연산은 num만 받아와서 하는 방향도 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

꼼꼼하게 봐주셔서 정말감사합니다
forEach 구문자체에서도 Number를 매번 호출한느데 심지어 체크할때마다 또 호출하네요...

},

validateContinuousDelimiters(inputText, escapedDelimiters) {
// 커스텀 구분자 정의 부분 제거
const textToValidate = inputText.replace(
REGEX_PATTERNS.CUSTOM_DELIMITER_DEFINITION,
'',
);
Comment on lines +30 to +33

Choose a reason for hiding this comment

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

와 저는 구분자 하나하나 비교를 수행했는데 모두 공백으로 바꾸는 방법이 있었네요!
좋은 방법인 것 같습니다

const allDelimiters = [...DEFAULT_DELIMITERS, ...escapedDelimiters];

allDelimiters.forEach((delimiter) => {
const escapedDelimiter = RegexUtils.escapeRegexChars(delimiter);
const continuousPattern =
RegexUtils.createContinuousPattern(escapedDelimiter);

if (continuousPattern.test(textToValidate)) {
throw new Error(ERROR_MESSAGE.DELIMITER_CONTINUOUS);
}
Comment on lines +41 to +43
Copy link

Choose a reason for hiding this comment

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

const validate = {
부터 이부분까지의 depth가 다소 깊은 것 같습니다!

depth를 줄이기 위해서 forEach부분도 따로 함수로 뺄 수 있을 것 같고
검증 로직도 아예 함수로 빼서 if문을 감출 수도 있을 것 같습니다 :)

Copy link
Author

Choose a reason for hiding this comment

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

말씀 감사합니다 이전주에 짠 코드임에도 잘 읽히지 않는것을 보니
가독성이 많이 떨어지게 작성한것 같습니다
depth를 줄이기 위해 방법을 추천해주셔서 감사합니다

});
},

validateDelimiters(customDelimiters) {
customDelimiters.forEach((delimiter) => {
// 빈 값 체크
if (!delimiter) {
throw new Error(ERROR_MESSAGE.DELIMITER_EMPTY);

Choose a reason for hiding this comment

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

예외 처리에 대해 알아보다가 커스텀 에러 객체에 대해 알게 되었습니다.
저도 문자열 계산기에서 일반 Error 객체를 사용했는데, 커스텀 에러 객체를 사용하면 어떤 에러가 발생했는지 명확히 알 수 있어서 좋을 것 같더라구요.
단순하게 궁금해서 여쭤보고 싶은데 커스텀 에러 객체 사용에 대해서는 어떻게 생각하시는지 여쭤보고 싶습니다!

Copy link
Author

Choose a reason for hiding this comment

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

Error 객체를 extends해서 쓰는것 너무 좋을 것 같은데요 ???
다만 코드와 파일이 늘어나다보니 확실하게 커스텀 애러 객체가 하는일을 명확히 해야겠군요
사용 좋은것 같습니다

}
// 공백 포함 체크
if (REGEX_PATTERNS.WHITESPACE.test(delimiter)) {
throw new Error(ERROR_MESSAGE.DELIMITER_HAS_WHITESPACE);
}
// 숫자 포함 체크
if (REGEX_PATTERNS.DIGIT.test(delimiter)) {
throw new Error(ERROR_MESSAGE.DELIMITER_HAS_NUMBER);
}
// 구분자가 한글자인지 체크
if (delimiter.length !== 1) {

Choose a reason for hiding this comment

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

아주 사소한 부분이지만요.. 1을 상수화하여 매직넘버를 최소화 하는 것은 어떠신가요?

Copy link
Author

Choose a reason for hiding this comment

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

너무 좋습니다!
허나 과도한 상수화가 오히려 가독성을 망칠 수 있다고 생각해서 고민이긴합니다
if (delimiter.length !== DELIMETER_MIN_LENGTH)
처럼 작성한다면 더 좋을것 같네요 수정에도 용이할것 같고요

throw new Error(ERROR_MESSAGE.DELIMITER_NOT_SINGLE_CHAR);
}
});
},
};

export default validate;
9 changes: 9 additions & 0 deletions src/view/InputView.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { Console } from '@woowacourse/mission-utils';

const inputView = {
async readLineMessage(message) {
const input = await Console.readLineAsync(message);
return input;
},
};
export default inputView;
8 changes: 8 additions & 0 deletions src/view/OutputView.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Console } from '@woowacourse/mission-utils';

const outputView = {
async printMessage(message) {
await Console.print(message);
},
};
export default outputView;
Loading