-
Notifications
You must be signed in to change notification settings - Fork 59
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
기본 계산기, 문자열 계산기 구현 및 단위테스트 구현 + AssertJ로의 리팩토링 #54
base: seojoonleee
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.
1주차 미션 수행하시느라 고생하셨습니다.
미션을 잘 수행해주셨네요.
커밋을 미션 단계별 또는 기능단위로 하면 더 좋을 것 같습니다.
테스트코드에서 많은 학습을 하셨겠네요!
고생하셨습니다.
import java.util.regex.Pattern; | ||
|
||
public class CalculateStringData { | ||
public static int add(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.
static으로 만든 이유가 있을까요?
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.
객채 생성 안 하고 그냥 CalcurateStringData.add 이런 식으로 바로 호출하려고 static으로 만들어보았습니다.
if (input.startsWith("//")) { | ||
int delimiterIndex = input.indexOf("\n"); // '\n'의 위치 찾기 | ||
if (delimiterIndex != -1) { | ||
delimiter = Pattern.quote(input.substring(2, delimiterIndex)); // 커스텀 구분자 추출 |
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 stringSum(numbers, delimiter); | ||
} | ||
|
||
private static int stringSum(String numbers, String delimiter) { |
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.
불필요한 인자를 전달한 것 같아요.
add 함수에서 만든 변수로 add 함수에서 아무 것도 하지않고 단순 전달만 진행하고 있어요.
어느 함수에 위치해야 할 변수인지 다시 생각해봐야 할 것 같네요.
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.
넵 두 변수가 add 내부에서만 쓰일 수 있도록 수정했습니다.
} | ||
|
||
// 음수 검증하는 메서드 | ||
private static void checkNumber(int number) { |
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.
좀 더 행위에 맞는 명확한 함수명을 사용하면 좋을 것 같아요.
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.
넵 수정했습니다.
public class Calculatorrrrr { | ||
public int add(int a, int b) { | ||
return a + b; | ||
} |
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.
가독성을 위해 간단한 코드더라도 메소드와 메소드사이는 띄어주는게 좋겠어요
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.
넵!
src/main/java/Calculatorrrrr.java
Outdated
try { | ||
return a / b; | ||
} catch (ArithmeticException e) { | ||
System.out.println("ArithmeticException(사용자가 0으로 나눴다는데요?)"); |
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.
디버깅 문장이 남아있는 것 같아요.
예외와 관련된 문장은 예외에 포함시켜주세요.
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.
넵
private final Calculatorrrrr calculator = new Calculatorrrrr(); | ||
|
||
@Nested | ||
public class TestingFunctions { |
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.
한 번 더 감싼 이유가 있나요?
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.
Nested 어노테이션을 사용해보고 싶어서 한 번 해봤습니다 ㅎㅎ
@Nested | ||
public class TestingFunctions { | ||
@Test | ||
@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.
테스트엔 displayname을 사용하지않고 메소드명 자체를 한글로 적기도 한답니다.
void 더하기() {
//
}
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.
옹 감사합니다
@DisplayName("0으로 나누면 ArithmeticException이 떠야해요") | ||
void divWithZero() { | ||
ArithmeticException ex = assertThrows(ArithmeticException.class, () -> calculator.div(2000, 0)); | ||
assertNull(ex.getMessage()); |
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.
여기도 앞서 언급했다시피 예외의 종류나 구체적인 예외메시지를 사용해서 검증해주면 좋을 것 같아요.
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.
문구 통일해서 assertEqual로 검증하도록 짜보았습니다.
미션을 하기 전 궁금했던 점과 수행하면서 느낀 점:
메인 메서드를 만들지 않고 프로그램을 돌려보라는 게 무슨 말이지? 그게 되나? 와 같은 생각이 테스트 코드를 만들고 실행해보기 전까지 계속 들었습니다. 그리고 기본 계산기를 구현한 후에 테스트 코드를 실행해보니 '아 테스트 코드란 게 이렇게 굴러가는 거구나 개발자들이 이미 어노테이션으로 다 만들어놓았구나 나는 그냥 가져다 쓰기만 하면 되겠구나' 하는 생각이 많이 들었던 것 같습니다.
그리고 Junit과 AssertJ를 병행하면서 둘의 차이를 느껴볼 수 있게끔 코스가 짜여져있는데 제가 이번 과제를 하면서 느껴본 바로는 JUnit은 뭔가 받는 인자가 두 개 이상으로 되어있어서 어느 게 예상값이고 어느 게 실제 값인지 타인이 봤을 때 아리까리할 수도 있을 것 같고, 그래서 AssertJ에선 깔끔하게 인자를 한 번씩 받게끔 하는 건가? 하는 생각이 들었습니다.
덕분에 깃허브 사용도 많이 하고 해서 이번 프리코스를 신청하길 잘 한 것 같아요 감사합니당