김성아 1주차 PR #58
김성아 1주차 PR #58
Conversation
There was a problem hiding this comment.
1주차 과제하시느라 고생하셨습니다! 피드백 몇가지 드릴게요
AssertJ를 사용하지 못했던것은
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
해당 임포트문을 쓰지 않았거나, gradle build를 하지 않아서 오류가 나지 않았나 추측해보고 있습니다.
오버플로우와 같은 예외처리를 꼼꼼히 해주신 부분 좋습니다. if문과 줄넘김부분에서 마치 알고리즘 문제를 푸는 듯한 느낌을 받았습니다ㅋㅋㅋ if문마다 연산으로 오버플로우를 체크하는 것 말고도 더 모듈화하여 처리하는 방법이 있을 것 같으니 한번 고민해보셔도 좋을 것 같습니다. (Math.addExact를 사용한다거나, long 혹은 비트부호 연산으로 오버플로우 처리 메서드를 분리한다거나)
어쨌든 1주차 고생하셨고, 새해복 많이 받으세요 👍
| if(a==0 || b==0) return 0; // overflow 체크 대비 | ||
| if(a>Integer.MAX_VALUE/b || a<Integer.MIN_VALUE/b) |
src/main/java/StringCalculrator.java
Outdated
| for(String n: str.split(delimiter)){ | ||
| if(n.isBlank()) continue; | ||
|
|
||
| if(ans <= Integer.MAX_VALUE - Integer.parseInt(n)) |
There was a problem hiding this comment.
여기서도 꼼꼼하게 오버플로우 처리를 잘 해주셨네요.
기존 사칙연산 계산기에 이미 오버플로우를 검수하는 기능을 만드셨으니, 해당 사칙연산 계산기를 불러와서 사용하셨어도 좋았을 것 같아요.
src/main/java/StringCalculrator.java
Outdated
| @@ -0,0 +1,39 @@ | |||
| public class StringCalculrator { | |||
There was a problem hiding this comment.
소소하지만 StringCalculrator → StringCalculator
r 오탈자가 있네요 🫠
| @DisplayName("문자열 계산기 테스트") | ||
| public class StringCalculratorTest { | ||
| @Test | ||
| public void NonDelimiterTest(){ |
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| public class AssertJTest { | ||
| public void NonDelimiterTest(){ |
src/test/java/AssertJTest.java
Outdated
|
|
||
| public class AssertJTest { | ||
| public void NonDelimiterTest(){ | ||
| StringCalculrator calcul = new StringCalculrator(); |
There was a problem hiding this comment.
메서드 호출마다 클래스를 호출하는 것보다 모든 메서드가 이용하는 것이라면 클래스에서 한번만 정의하는게 더 나을 수도있습니다.
| } | ||
| } |
| @@ -0,0 +1,17 @@ | |||
| public class FourBasicArithmaticOperations { | |||
There was a problem hiding this comment.
클래스명을 다른 사람들도 이해가 잘 되게 수정하면 좋을 것 같아요!
FourBasicArithmaticOperations이라는 이름을 보고 계산기라는 것이 바로 안 떠올라서요!
Calculator, SimpleCalculator 등과 같은 이름은 어떨까요?
There was a problem hiding this comment.
아하 사칙연산 계산기를 번역해 나온 이름이었는데 더 직관적인 단어가 있었군요! 반영하여 수정하겠습니다.
| public int divide(int a, int b){ | ||
| return a/b; |
| assertThrows(RuntimeException.class, ()->calcul.strPlus("1000000000,2000000000,3000000000")); | ||
| assertThrows(RuntimeException.class, ()->calcul.strPlus("!!,@@")); | ||
| assertThrows(RuntimeException.class, ()->calcul.strPlus("-1,-2")); | ||
|
|
There was a problem hiding this comment.
예외가 발생했을 때, 예외 메세지를 추가하면 디버깅에 용이할 것 같아요! 편의에 따라 달라질 수 있겠지만,
저는 예외처리할 때 메세지를 항상 표시하는 편이어서요! 참고만 해주세요😉
기초적인 틀을 구현한 후, 각 케이스를 생각하여 예외처리/기능추가하는 방식으로 진행하였습니다.
IDE는 vscode를 이용하였는데, 환경을 설정하는 부분에서 어려움을 겪었습니다.
Junit은 제가 만든 프로젝트에서는 정상 작동하였지만 clone으로 가져온 프로젝트에선 어째선지 작동만 되고 작동결과가 보이지 않아 테스트 프로젝트와 clone한 프로젝트끼리 코드를 옮겨가며 코드를 작성하였습니다.
AssertJ는 build.gradle에 제대로 의존성을 추가하였음에도 에러가 발생하여 실행해보지 못했습니다.
(실행 시
the import org.assertj cannot be resolved
The method assertThat(int) is undefined for the type AssertJTest
The method assertThatThrownBy(()->{}) is undefined for the type AssertJTest
라는 에러가 해당 함수를 사용한 모든 라인에서 발생하였습니다.
의존성에 무언가 문제가 있어 AssertJ불러오기를 하지 못하는 것 같은데 해결하지 못했습니다)
테스트파일을 작성하면서 코드를 수정하다보니, 테스트파일의 기능들을 한정적으로 사용한 것 같아 아쉽습니다. 조금 더 다양한 결과값이 있었거나 assertEquals이나 Throws 말고도 다른 메소드를 사용할 수 있었을 것 같습니다. input값이 단순했다면 값을 랜덤으로 넣아가며 예상치 못한 오류가 있진 않는지 확인해볼 수도 있을 것 같다는 생각을 했습니다.