Skip to content

[문자열 계산기] 박현규 미션 제출합니다.#4

Open
ParkHyeonkyu wants to merge 7 commits into
Java-JavaScript-Language-Stuty:mainfrom
ParkHyeonkyu:ParkHyeonkyu
Open

[문자열 계산기] 박현규 미션 제출합니다.#4
ParkHyeonkyu wants to merge 7 commits into
Java-JavaScript-Language-Stuty:mainfrom
ParkHyeonkyu:ParkHyeonkyu

Conversation

@ParkHyeonkyu

@ParkHyeonkyu ParkHyeonkyu commented Feb 7, 2025

Copy link
Copy Markdown

1. 신경써서 구현한 부분

  • 계산 방식
    .split()을 사용해서 입력 문자열을 배열로 바꾸고
    .reduce() 과정에서 NaN을 검사하는 형식으로 덧셈 및 예외 처리 기능을 구현해보았습니다.
  • 커스텀 구분자
    if문과 startsWith을 사용해 input 이 //로 시작할 경우,
    즉 커스텀 구분자가 존재 할 경우에 코드가 어떤 식으로 작동할지 구현하고자 했습니다.
    /n이 시작되는 인덱스를 search로 찾고, slice를 활용해서 //과 \n사이에 있는 커스텀 구분자를 추출했습니다.
    그 과정에서 Console.print("입력값 (JSON): " + JSON.stringify(input));로 확인한 input의 \n은 컴퓨터가 개행 문자가 아닌 ‘문자 그대로의 \n’을 나타내고 있고, const delimiterEndIndex = input.search(/\n/); 에서 내가 지정한 \n은 ‘개행 문자 \n’이었음을 알게 되었습니다.
    따라서 input으로 받은 \n(문자 그대로의 값)을 replace를 사용해서 \n(개행 문자)로 변경시켰고,
    이러한 비슷한 오류로 정규식에서 특정 패턴을 의미할 수 있는 여러 기호들을 이스케이프 처리하기 위해
    const escapeRegExp = (str) => str.replace(/[-/\^$*+?.()|[]{}]/g, '\$&');행을 추가했습니다.
    그리고 // \n안에 동일한 구분자는 하나만 들어갈 수 있도록 했습니다.
  • 소수점
    소수 계산을 할지 말지 고민했었는데 할 수 있도록 만들어보았습니다.
    따라서 온점(.)은 커스텀 구분자로 사용할 수 없도록 했습니다.

2. 피드백이 필요한 부분

  • 음 코드 길이가 좀 길어진 것 같아서 아쉽습니다
    더 간단히 하는 방법도 있을 것 같은데
    다른 분들은 어떤 방식으로 해결하셨는지 궁금합니다!

3. 내가 고려한 예외

  1. 숫자와 유효 구분자 이외의 문자(한글, 영어)
  2. 잘못된 커스텀 구분자 설정 형식
    2-1) //로 시작해서 \n로 끝나지 않는 경우
    2-2) 중복된 구분자를 사용하는 경우 ex) //**\n
  3. 유효하지 않은 구분자 사용 ex) //*\n1,2#3
  4. 음수 사용 ex) -1,-2,3
  5. 소수점을 커스텀 구분자로 사용하려는 경우

Console을 활용하여 사용자에게 문자열을 입력받는 기능 구현
commit 테스트
split과 reduce를 활용하여
기본 구분자(,:)를 기준으로 문자열 덧셈 구현
//과 \n사이에 구분자를 커스텀 구분자로 설정하는 기능 구현
try-catch문을 활용해서 예상되는 5~6개의 예외를 처리하는 코드 구현
미션 출력 조건에 맞도록 Console.print부분을 주석처리
@ParkHyeonkyu ParkHyeonkyu changed the title [문자열 계산기} 박현규 미션 제출합니다. [문자열 계산기] 박현규 미션 제출합니다. Feb 7, 2025
가독성 향상을 위한 주석 제거
Comment thread src/App.js
delimiter = new RegExp(`[,:${escapeRegExp(customDelimiter)}]`);
} else if (!(input.startsWith("//") || !isNaN(Number(input[0])))) {
throw new Error("양수 또는 //로 시작해야 합니다.");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

길이가 길어서 가독성이 떨어진다면 검증 과정의 책임을 분리해도 좋을 것 같습니다.

Comment thread src/App.js Outdated
throw new Error("유효하지 않은 문자가 포함되어있습니다.");
}, 0);

return sum;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이 부분도 코드가 복잡해지는 것을 완화하기 위해서 의도적으로 검증과 연산의 책임을 분리할 수 있을 것 같아요!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

동의합니다! 예외는 예외끼리 묶고, 유효한 숫자들 끼리만 합산하면 좀 더 짧게 코드를 짤 수 있을 것 같습니다!
const num = Number(cur);
if (isNaN(num)) throw new Error("유효하지 않은 문자가 포함되어있습니다.");
if (num < 0) throw new Error("음수를 입력할 수 없습니다.");
return acc + num;
이런 식으로 바꾸면 좀 더 간단해질 것 같아요

@yuninam2128 yuninam2128 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

제 실력에는 코드 읽기도 너무 어렵지만..피드백이 필요하시다고 말씀하신 부분 리뷰헤보았습니다! 수고하셨습니다!!

Comment thread src/App.js Outdated
}
if (customDelimiter === ".") {
throw new Error(".은 구분자로 사용할 수 없습니다.");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

let customDelimiter = input.slice(2, delimiterEndIndex);
if (/(.)\1+/.test(customDelimiter) || customDelimiter === ".")
throw new Error("중복된 구분자 또는 '.'은 사용할 수 없습니다.");
이런 식으로 묶어주면 코드가 좀 더 간결해질 수 있을 것 같습니다.

Comment thread src/App.js Outdated
throw new Error("유효하지 않은 문자가 포함되어있습니다.");
}, 0);

return sum;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

동의합니다! 예외는 예외끼리 묶고, 유효한 숫자들 끼리만 합산하면 좀 더 짧게 코드를 짤 수 있을 것 같습니다!
const num = Number(cur);
if (isNaN(num)) throw new Error("유효하지 않은 문자가 포함되어있습니다.");
if (num < 0) throw new Error("음수를 입력할 수 없습니다.");
return acc + num;
이런 식으로 바꾸면 좀 더 간단해질 것 같아요

Comment thread src/App.js Outdated
}
if (customDelimiter === ".") {
throw new Error(".은 구분자로 사용할 수 없습니다.");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

예외 처리를 정말 섬세하게 잡으신 것 같습니다bbb
특히 .을 구분자로 처리하시지 못하도록 처리하신 부분이 소수까지 덧셈 고려를 하신 것 같아 고민을 많이 하신 것 같습니다. 코드 길이는 길지만 프로그래밍 언어에 익숙하지 않은 사람들이 보기에(저같은ㅎ) 직관적인 코드여서 저는 읽기 편한 것 같습니다!!

Comment thread src/App.js Outdated
const delimiterEndIndex = input.search(/\n/);

if (delimiterEndIndex === -1) {
throw new Error("\\n이 필요합니다.");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

왜 \n이 필요한 지 알려주실 수 있나요?

  • 사용자가 \n이 필요하다는 warning을 이해할 수 있을지...? 예를 들어 미션의 구현 조건은 '앞의 기본 구분자(쉼표, 콜론) 외에 커스텀 구분자를 지정할 수 있다. 커스텀 구분자는 문자열 앞부분의 "//"와 "\n" 사이에 위치하는 문자를 커스텀 구분자로 사용한다.' 이렇게 되어 있어서 왜 \n이 필요한지 의문이 들 것 같아요

Comment thread src/App.js Outdated
let customDelimiter = input.slice(2, delimiterEndIndex);
if (/(.)\1+/.test(customDelimiter)) {
throw new Error("중복된 구분자를 사용할 수 없습니다.");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

중복된 문자 자체가 구분자이면 어떡하나요? 예를 들면 ";;" 자체가 구분자로 지정하고 싶으면...?

Comment thread src/App.js Outdated
throw new Error("중복된 구분자를 사용할 수 없습니다.");
}
if (customDelimiter === ".") {
throw new Error(".은 구분자로 사용할 수 없습니다.");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

소수 계산을 고려하셔서 이렇게 표현하신 것 같은데 자바 스크립트는 숫자를 64비트 부동 소수점 형식으로 저장하기 때문에 큰 숫자를 저장할 경우에 오버플로우, 작은 숫자를 저장 경우에 다운 플로우가 발생해서 부정확한 계산 결과를 낼 수도 있다고 합니다. 이러한 오류는 어떻게 처리하시는 지 궁금합니다.
서치를 해보니 toFixed() 메서드를 사용하여 소수점 이하 자릿수를 제한하는 방식을 사용할 수 있다고 합니다. 제가 자바스크립트를 잘 모르기도 하고 업데이트 등의 이유로 위의 사실이 틀릴 수도 있으니 교차 검증을 꼭 해보세요..!

깃허브 코드리뷰의 피드백을 반영한 리팩토링 코드입니당
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.

4 participants