-
Notifications
You must be signed in to change notification settings - Fork 83
[김민지] 3단계 - 문자열 계산기 구현 #78
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: mmm307955
Are you sure you want to change the base?
Conversation
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.
안녕하세요 민지님! 3단계까지 한 번에 진행해 주셨네요! 👍👍
요구사항을 놓친 것은 없지만, 테스트 코드가 누락되거나 코드에 버그가 존재하는 것 같아요.
해당 사항 반영하시고 다시 요청 부탁드립니다!
추가로 커밋에는 의미 있는 작업만 하는 것을 권장드려요!
만약 실수로 인해 놓치거나 포함되지 않아야 할 사항이 커밋되었다면, amend, reset 같은 깃 명령에 대해 학습하시면 좋을 것 같아요! (명령어를 사용할 필요는 없고, IntelliJ git GUI를 사용하면 쉽게 사용할 수 있어요!)
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.
1단계 요구사항인 초간단 계산기에 3단계 요구사항인 문자열 계산기 기능을 추가해주셨네요!
미션의 요구사항에는 별도의 파일로 만들라고 되어 있지 않았기에 이렇게 구현해도 상관은 없을 것 같아요!
다만, 사칙연산을 하는 계산기의 기능은 계산기가 하는 행동(메서드)이라고 쉽게 생각할 수 있지만, 문자열 계산기 같은 특수한 목적을 가진 행동은 과연 Calculator
가 가질 수 있는 행동이 맞을까요?
@Test | ||
void testCalculateString(){ | ||
int result = c.calculateString("1,2:3"); | ||
assertEquals(6,result); | ||
result = c.calculateString(""); | ||
assertEquals(0,result); | ||
result = c.calculateString("1,2"); | ||
assertEquals(3,result); | ||
result = c.calculateString("1,2,3"); | ||
assertEquals(6,result); | ||
} |
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.
3단계 미션의 요구사항 중 하나인 문자열 계산기 테스트 코드네요!
다만 이 테스트 코드만 보았을 때는 잘 작성된 테스트 코드와는 거리가 있어 보여요. 😂
해당 규칙을 지켜서 수정해주세요!
assertEquals
과 같은 검증부를 하나만 사용한다.- 테스트가 수행하는 목적을 이름으로 나타낸다. (
@DisplayName
)
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.
또한 요구사항 중 하나인 커스텀 구분자에 대한 테스트가 누락되어 있는것 같아요!
return a / b; | ||
} | ||
|
||
public int calculateString(String input) { |
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.
input으로 "1"과 같은 길이가 하나인 문자열을 넣으면 결과가 어떻게 나올까요?
String separator = "[,:]";//정규표현식. split()메소드에서 사용된다. '[]안의 문자중 하나와 일치'라는 의미를 갖는다. | ||
String numbersStr = input; | ||
|
||
if (input.substring(0, 2).equals("//")) { | ||
int slashStartIndex = input.indexOf("//"); | ||
int newLineStartIndex = input.indexOf("/n"); | ||
|
||
if (newLineStartIndex != -1) { | ||
String customSeparator = input.substring(slashStartIndex + 2, newLineStartIndex); | ||
separator = "[,:" + customSeparator + "]"; | ||
numbersStr = input.substring(newLineStartIndex + 1);//개행문자는 하나의 문자로 취급. | ||
} | ||
} | ||
|
||
String[] numbers = numbersStr.split(separator); | ||
|
||
int sum = 0; | ||
for (String number : numbers) { | ||
if (!number.isEmpty()) { | ||
int value = Integer.parseInt(number);//String 배열에 있는 numbers를 하나씩 int형으로 변환 | ||
if (value < 0) { | ||
throw new RuntimeException("음수는 사용할 수 없습니다 : " + value); | ||
} | ||
sum += value; | ||
} | ||
} | ||
return sum; | ||
} |
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.
의미를 가지는 변수명 👍👍
계산기 과제 수정해서 제출했습니다!
3단계까지 해서 올려야했는데, 급하게 알게돼서 뒤늦게라도 제출해봅니다😭
제가 스스로 코드를 작성하진 못했지만, 정규표현식같이 제대로 알지 못했던 부분을 공부해서 저에게는 의미가 있었습니다...!
너무 늦게 제출해서 다음부터는 미리 잘 제출해서 코드리뷰를 받아보도록 하겠습니다.!!