-
Notifications
You must be signed in to change notification settings - Fork 156
[4기 - 황창현] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #124
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
[4기 - 황창현] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #124
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.
안녕하세요 창현님! 걱정하셨던 것보다 잘하고 계신것 같습니다! 👍
다만, 전반적으로 일반적인 자바 컨벤션을 적용하실 필요가 있어보입니다! 인텔리제이를 활용하시면 편리하게 적용하실 수 있습니다.
지금은 혼자 개발하고 있지만 추후에 팀 프로젝트에 가서는 컨벤션과 가독성이 매우 중요합니다!
그리고 질문에 대한 답변을 드리면
각 객체에 대한 책임이 적절하게 이루어진 것 인지, 객체의 책임을 더 세분화시켜서 나눠야하는지 궁금합니다.
- 코드가 길어지고 복잡해진다면 객체가 너무 많은 책임을 가지고 있는지 생각해보고 좀 더 세분화하는 것을 고려할 것 같습니다!
입력 값을 받을 때 원하는 값이 아닌 경우 에러를 throw를 하여 프로그램을 멈추게 하는 것이 맞는지 궁금합니다.
→ 저의 경우 원하는 값이 아닐 경우 따로 에러를 throw하지 않고 잘못 입력되었다고 안내해주고 다시 입력 값을 받을 수 있도록 구현했습니다. 이것은 본인이 생각한 로직에 맞게끔 작성하면 되는걸까요?
- 사용자 입장에서 생각해시면 좋을 것 같습니다! 잘못 입력했다고 프로그램이 종료가 된다면 안정적인 프로그램이라고 할 수 있을까요?
강의에서 나오는 테스트 코드만 작성해보고, 스스로 작성한 테스트 코드가 처음입니다. 테스트 코드 작성하는 것이 굉장히 어렵다고 느껴지는데 이렇게 작성하는 것이 맞을까요?
- 성공 테스트뿐만 아니라 실패 테스트까지 작성해보시는 게 좋습니다. 해당 영상 보시길 권장해드립니다~
제가 작성한 코드에서 조금 더 객체지향적으로 리팩토링하기 위해 참고하면 좋을 무언가가 있을까요?
- 객체지향 체조 원칙를 적용해시는 건 어떨까요?
|
||
public class Operator { | ||
|
||
static public int plus(int a, int 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.
앗,, 기본적인 실수를 했네요! 확인해서 모두 수정해보겠습니다!! :)
import org.programmers.java.repository.FormulaRepository; | ||
|
||
public class Calculator { | ||
private boolean exitStatus = true; |
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.
exitStatus
라는 네이밍만 봤을 때는 종료 상태라고 해석이 되어 헷갈릴 여지가 있어 보입니다!
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.
말씀해주신거 보고 프로젝트를 보니 아직 네이밍을 하는 능력이 많이 부족한 것 같아요! 한 번 다시 고민해보면서 변경할 수 있는 부분은 수정해보도록 하겠습니다!
String inputNum = input.numInput(); | ||
output.selectMsg(inputNum); | ||
switch (inputNum) { | ||
case "1": |
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", "2" 와 같이 단순한 숫자로 표현하면 어떤 의미를 가지고 있는지 보여지지가 않는 것 같아요. 매직넘버에 대해 공부하고 이를 개선하면 좋을 것 같아요!!
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.
확실히 치환하게되면 의미부여가 되어 눈에 더 잘 읽히는 코드가 될 것 같아요! 매직넘버에 대해서 공부해보고 변경해보도록 하겠습니다!!!
else { | ||
while(operatorDeque.size() > 0){ | ||
if(operatorPriority.get(operatorDeque.peekLast()) <= operatorPriority.get(item)) { | ||
operatorDeque.add(item); |
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.
depth가 너무 길다고 생각이 듭니다! 메서드나 객체로 분리하는게 좋을 것 같아요!
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.
코드가 좀 길고 반복문과 if문이 많다고 느껴졌었는데 확실히 지금 다시보니 depth가 많이 깊네요. 최대한 메서드랑 객체로 분리해보도록 하겠습니다!
|
||
import java.util.*; | ||
|
||
public class Calculate { |
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 enum ErrorMsg { | ||
CALCULATE_VALIDATION_ERROR_MSG("잘못된 입력 값 입니다. 형식을 맞춰주세요. ex) 2 + 4 ..."), | ||
SELECT_VALIDATION_ERROR_MSG("잘못된 입력 값 입니다. 1-3 사이의 숫자를 선택해주세요"); |
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.
ERROR_MSG
이 부분은 중복된 표현이라고 생각이 듭니다. 그리고 어떤 오류인지 명확하게 알 수 있도록 나타내는게 좋을 것 같은데 어떻게 생각하시나요?
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.
지금 보니 확실히 어떠한 상황에 대한 에러 메시지인지 한 눈에 보기 어려운 것 같습니다! 명확하게 나타내고 중복된 표현은 제거 해보겠습니다!!
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
public class Validation { |
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.
Validation
보다 Validator
가 더 적절할 것 같은데 어떻게 생각하시나요?
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.
단순히 검증이라고 생각했었는데 소프트웨어적으로 보면 Validator 검증 도구가 더 맞는 표현 같습니다! 네이밍에 대해서는 다시 한 번 생각해보고 작성할 수 있도록 해보겠습니다!
|
||
switch (item){ | ||
case "+": | ||
numberDeque.add(String.valueOf(Operator.plus(num1, num2))); |
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.
Operator
가 단순히 연산의 기능만을 위해 객체로 분리한거 라면 좀 더 생각해볼 필요가 있을 것 같습니다! 해당 글을 참고하시면 좋을 것 같습니다!
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.
다른 PR에서 Enum을 통해 Operator 기능 구현을 하는 것을 보고 제 스스로 한 번 만들어볼 수 있는 방법이 어떤 것인지 생각해보다가 이런식으로 작성하게되었습니다! 확실히 링크 걸어주신 글을 보면 더 깔끔하게 리팩토링 할 수 있을 것 같아요! 참고해서 수정해보겠습니다!
private final Map<String, Integer> operatorPriority = Map.of( | ||
"+", 1, | ||
"-", 1, | ||
"*", 2, |
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.
우선 순위라고 보면 2번이 더 후순위라고 생각이 되네요!
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.
저도 이것에 대해서 고민을 많이했었는데 확실히 주성 서브멘토님께서 말씀하신 것 처럼 반대로 표현하는 것이 더 올바른 표현 법 같네요! 이것도 수정해보도록 하겠습니다!!
import java.util.Map; | ||
|
||
public interface Output { | ||
void menuMsg(); |
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.
interface를 사용한 이유가 궁금합니다!
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, Output은 현재 CONSOLE 형식으로 작성되지만 추후에 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.
마땅히 추상화해야할 부분이 없다면 안하셔도 괜찮습니다! 오히려 인터페이스를 잘못 사용할 경우 변경에 취약해질 수 있습니다!!
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.
안녕하세요 창현님:)
많이 고민한 흔적이 보이네요! 💯
전체적으로 잘 짜주셨으나 몇가지 피드백 드릴 것들이 있어서 남겨놨습니다.
확인 부탁드려요~
코드 리뷰 외에 궁금한 것이 있는데, Readme를 보게 되면 요구사항과 구현 내용에 요구사항에 대한 내용이 없는 것 같아요. 그리고 commit에 대한 설명을 적어주셨는데, 커밋이 100개가 된다면 그 설명을 다 적는게 가능할까요?
README에 어떠한 내용들을 담아야하는지에 대한 고민을 해보셨으면 좋겠습니다 :)
창현님이 주신 질문에 대한 답변을 해보자면,
- 각 객체에 대한 책임이 적절하게 이루어진 것 인지, 객체의 책임을 더 세분화시켜서 나눠야하는지 궁금합니다.
객체지향을 생각할 때, 객체를 먼저 나누기 보다는 이 프로젝트에서 어떠한 행위(메시지)들이 필요할지 먼저 고민해보고 객체를 나눠보면 더 좋은 객체지향이 될 수 있을 것 같아요.
- 입력 값을 받을 때 원하는 값이 아닌 경우 에러를 throw를 하여 프로그램을 멈추게 하는 것이 맞는지 궁금합니다.
→ 저의 경우 원하는 값이 아닐 경우 따로 에러를 throw하지 않고 잘못 입력되었다고 안내해주고 다시 입력 값을 받을 수 있도록 구현했습니다. 이것은 본인이 생각한 로직에 맞게끔 작성하면 되는걸까요?
이건 요구사항마다 다를 것 같아요.
throw를 통해 프로그램을 종료 시키는 요구사항이 있을 수 있고, 예외가 발생하면 예외 메시지를 보여준 후에 다시 입력을 받을 수 있는 요구사항이 있을 수 있겠네요.
과제에서 주어진 요구사항이 없다면, 본인이 이유를 가지고 개발하시면 될 것 같습니다 :)
- 강의에서 나오는 테스트 코드만 작성해보고, 스스로 작성한 테스트 코드가 처음입니다. 테스트 코드 작성하는 것이 굉장히 어렵다고 느껴지는데 이렇게 작성하는 것이 맞을까요?
부족한 부분이 있어서 테스트 코드 부분에 코멘트 남겨드렸습니다.
- 제가 작성한 코드에서 조금 더 객체지향적으로 리팩토링하기 위해 참고하면 좋을 무언가가 있을까요?
우선 아무래도 객체지향 관련 책을 읽어보시는게 좋겠죠?
객체지향의 사실과 오해, 오브젝트 이 두 책 추천드립니다!
그리고 객체지향 생활체조원칙 이라는 것이 있습니다.
아래 내용을 지키면서 개발하도록 노력해보세요.
private final Calculate calculate; | ||
private final FormulaRepository formulaRepository; | ||
|
||
Calculator(Input input, Output output, Calculate calculate, FormulaRepository formulaRepository){ |
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.
Calculator에 너무 많은 파라미터를 가지고 있는 것 같아요.
파라미터는 1, 2개 이내로 가질 수 있도록 해보시면 좋을 것 같아요.
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.
말씀해주신 객체지향 생활체조원칙 중 하나에 포함되는 것을 확인했습니다! 어떤식으로 줄여야할지 한 번 고민해보고 적용 해보도록 하겠습니다!!
import java.util.Map; | ||
import java.util.Scanner; | ||
|
||
public class Console implements Input, Output { |
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 Validation validation; | ||
private final Scanner scanner = new Scanner(System.in); | ||
|
||
public Console(Validation validation) { |
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.
Console이 Validation을 의존하는 구조가 좋지 않아보입니다.
화면을 보여주는 역할을 하는 Console에 유효성 검사를 하는 Validation이 필요할까요?
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.
처음에는 Console에서 입력받은 후 바로 입력값에 대한 검증이 필요하다고 단순하게 생각해서 의존하는 구조로 만들었던 것 같습니다. 하지만 리팩토링하려고보니 Console에서 Validation을 의존하고 있어 Validation에서 콘솔을 사용하기 어려웠고 View단에 검증 기능이 있는 것이 역할과 책임 분리가 제대로 되지 않은 구조라는 것을 느꼈습니다! 말씀해주신 것 처럼 Console을 싱글톤으로 변경해서 주입하는 방식으로 변경하고 의존 관계를 없애보도록 하겠습니다!!
SELECT_NUM_MESSAGE("선택 : "), | ||
EXIT_INFO_MESSAGE("프로그램이 종료되었습니다"); | ||
|
||
|
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.
리팩토링 하면서 불필요한 개행들도 한 번 씩 다시 체크해보겠습니다! 감사합니다!
import java.util.stream.Collectors; | ||
|
||
public class FormulaMemoryRepository implements FormulaRepository { | ||
Map<Long, String> formulaMap = new HashMap<>(); |
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.
hashMap을 사용하게 되면 장, 단점이 어떤게 있을까요?
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.
hashMap은 데이터� 저장시 해시함수를 통해 해시코드를 얻어 버킷에 저장하는 방식으로 알고 있습니다! 그래서 리스트 보다 데이터 검색에 빠르다는 장점과 key값이 중복되지 않는 점 등이 있습니다. 단점으로는 순서가 보장되지 않는다는점인데 과제에서 계산된 이력에 대한 것은 순서가 보장되는게 맞다고 생각이 들었고 팀원분들이 LinkedHashMap이 순서보장이 된다고 알려주셨습니다!! LinkedHashMap을 통해 순서를 보장시키는 쪽으로 코드를 리팩토링하려고 합니다.
Map을 사용할 때 key를 index로하고 value를 연산식 + 결과값으로 저장하게되면 출력시 어차피 순차적으로 출력할 것인데 Map을 사용하면 Iterator를 사용해야되는 비용이 들어서 오히려 Map을 사용하는 것이 맞나? 그냥 리스트를 사용하면 되지 않나? 라는 생각도 들었습니다. 그렇다고 key를 연산식, value를 결과값으로 하는 경우 Key인 연산식이 중복되는 경우 계산된 이력에서 중복 문제로 저장되지 않기 때문에 해당 이력은 2개가 아닌 1개로 남는 것도 문제가 될 것 이라고 생각하는데,, 이 부분이 잘 이해되지 않는 것 같습니다.
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.
저도 리스트로 해도 충분하다고 생각합니다. 다만, 요구사항이 Map을 활용하라고 되어 있으니 key를 index로하고 value를 연산식 + 결과값으로 저장하는 방식으로 구현해도 될 것 같아요!
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.
의견 감사드립니다!!! :)
|
||
if(formulaList.size() != 0) afterCheck = checkFormulaValidation(formulaList); | ||
|
||
if(!afterCheck) System.out.println(ErrorMsg.CALCULATE_VALIDATION_ERROR_MSG.getErrorMsg()); |
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.
println을 Validator에서 사용하고 계신데, 화면에 노출시켜주는 책임을 Output에 위임하신거 아닌가요?
현재 구조처럼 Output을 사용하지 않고 print를 찍어준다면 사실상 Output을 만든 이유가 없을 것 같다는 생각이 드네요.
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.
Output에서 모든 것을 처리할 수 있게끔 책임을 위임했었는데 구조를 잘못가져가서 print문으로 대체해서 작성했었습니다! 그때는 어떻게 할지 잘 모르겠었으나 말씀해주신 콘솔을 싱글톤 객체로 변경하고 의존관계를 다르게해서 구조를 다시 잡으면 원래의 취지에 맞게 작성할 수 있을 것 같습니다! 다음번 PR때 반영하겠습니다!
|
||
for(String str : splitMsg){ | ||
if(str.matches(numberMatch)) formulaBeforeValidation.add(str); | ||
else if(str.matches(operatorMatch)) formulaBeforeValidation.add(str); |
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.
else문, switch문을 사용하지 않고 구현하도록 연습해보세요!
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.
최대한 else if, else, switch문을 사용하지 않고 구현해보도록 하겠습니다!!
|
||
List<String> formulaList = formulaSplitValidation(inputMsg); | ||
|
||
if(formulaList.size() != 0) afterCheck = checkFormulaValidation(formulaList); |
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.
formulaList.size() != 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.
가독성이 떨어지는 코드들은 모두 메소드로 빼볼 수 있도록 하겠습니다!
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
public class CalculateTest { |
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.
예외에 대한 예측을 제대로 해야 프로그램이 잘못 돌아가거나 멈추는일을 방지할 수 있기 때문이라고 생각합니다! 예외에 대해서도 한 번 더 공부해보고 반영해볼 수 있도록 하겠습니다!!
@DisplayName("중위 표기식 -> 후위 표기식 변경") | ||
void infixToPostfix(){ | ||
// given | ||
String[] formulaList = {"3", "+", "5", "*", "10"}; |
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.
Guide to JUnit 5 Parameterized Tests
해당 글 보시고 한번 공부해보시죠 ㅎ
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.
감사합니다!! 보고 테스트 코드 다시 한 번 작성해보도록 하겠습니다!!
안녕하세요 네이비 멘토님, 주성 서브멘토님 우선 바쁘심에도 불구하고 하나하나 꼼꼼하게 리뷰해주셔서 감사드립니다 👍 👍 말씀해주신 피드백들을 최대한 반영해서 커밋하고 누락된 요구사항과 구현 내용, 피드백 반영 사항을 PR 내용에 수정해놓았습니다! 아무래도 자바 컨벤션에 대한 이해, 객체지향 체조 원칙에 대한 개념이 없어서 그런지 고칠게 많아서 피드백을 반영하는데 시간이 오래걸렸던 것 같습니다! 자바 컨벤션과 객체지향 체조 원칙에 초점을 맞춰서 리팩토링 하려고 했고, 한 객체에 책임이 많은 것 같아서 여러 객체로 분리하여 책임을 나누고 depth를 최대한 2이하로 줄이려고 노력했습니다! 그리고 피드백을 반영하면서 가장 힘들었던 점은 하나를 고쳤는데 연쇄적으로 고쳐야하는 상황이 여러번 발생했던 점인 것 같아요. 리팩토링하면서 객체지향적으로 설계하지 못했다고 많이 느껴서 그런지 아래와 같이 질문드리고 싶습니다!
우선 제가 이러한 문제가 발생된 이유는 요구사항에 대해 명확하게 정의 내리지 않고 코드로 미리 작성하려고 했던 점이 가장 잘못되었던 것 같습니다. 😢😢 리팩토링 한 코드들도 다시 한 번 잘 부탁드리며 부족한 부분을 많이 솔직하게 얘기해주셔도되니 가감없이 피드백 부탁드립니다 🙏🙏 항상 감사드립니다 👍 👍 😄 😄 |
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.
@Hchanghyeon
안녕하세요. 창현님 리팩터링하시느라 정말 고생많으셨습니다~ 👍
피드백을 잘 반영하신 것 같아요~! 안정적인 프로그램을 만들기 위해서 예외처리
, 실패 테스트
를 추가하시면 더 좋을 것 같습니다 😄
질문 주신 내용에 답변하자면
처음 코드를 작성할 때 나중에 리팩토링 하더라도 많은 로직을 바꾸지 않고 작성할 수 있는 Tip 같은게 있을까요?
-
바꿔서 말하면 객체지향적으로 설계하는 법을 물어보신 것 같아요! 말씀하신 것 처럼 코드 작성 이전에 요구사항을 바탕으로 설계를 해보시면 도움이 될 것 같습니다! 그림 혹은 UML을 그려보시는 건 어떨까요?
-
그리고 아직 경험이 많지 않기 때문에 처음부터 완벽하게 설계하는 것은 매우 어려운 일입니다. 피드백을 통해 지속적으로 개선하는 방향으로 가면 좋을 것 같아요!!
☺️
|
||
public class App { | ||
public static void main(String[] args){ | ||
Calculation calculation = new Calculation(); |
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.
calculation
명칭만 보고는 어떤 동작을 하는 객체인지 모르겠습니다! 좀 더 객체를 잘 대변할 수 있는 이름이면 좋을 것 같습니다! (DTO가 아니라면 동사로 표현하는게 좋습니다.!)
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.
계산하는 부분에 대한 로직을 담는다고 생각하여 계산이라고 지은 것 같습니다! 다시 보니 너무 포괄적인 의미를 담고 있어서 변경하는게 좋을 것 같습니다! DTO가 아닌 객체들은 메소드처럼 동사로 표현하는 것이 좋을까요?! 객체는 뭔가 명사로 지어야된다는 말을 많이 들었어서요!
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.
@Hchanghyeon 아! 제가 잘못 전달했네요! 동사가 아니고 명사가 맞습니다..😅 다만 객체인 만큼 어떤 동작을 하는 주체로 표현하면 좋을 것 같아요!
this.output = Console.getInstance(); | ||
this.calculation = calculation; | ||
this.formulaRepository = formulaRepository; | ||
this.validator = new Validator(); |
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.
Validator
만 객체를 직접 생성하신 이유가 궁금하네요!
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.
우선 네이비 멘토님께서 객체를 생성할 때 매개변수로 들어가는 인스턴스 수를 줄여달라고 말씀하셔서 생성자 객체에서 일부는 new로 전환해야겠다는 생각이 들었습니다! 그 중 메인 로직인 계산과 저장소의 기능보다는 검증을 new로 생성하는 것이 좋을 것 같아서 Validator만 new로 생성했습니다!!.. 근데 매개변수로 들어가는 인스턴스의 수를 줄여야할 때 어떤 방식으로 줄여야할지 감을 못잡겠습니다. 지금처럼 Calculator가 컨트롤러의 역할을 하고 있어서 모든 인스턴스들을 주입받는 형식으로 있다보니,, 어떤식으로 줄여야할지 잘 모르겠습니다!!
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.
우선 Validator가 말씀해주신대로 유틸클래스로 사용하면 될 것 같아서 static으로 변경처리하여 삭제처리했습니다!
compareFormula(getFormula, inputFormula, result); | ||
} | ||
} | ||
|
||
private void compareFormula(String getFormula, String inputFormula, String result){ | ||
StringBuilder stringBuilder = new StringBuilder(); | ||
stringBuilder | ||
.append(inputFormula) | ||
.append(" = ") | ||
.append(result); | ||
String formula = stringBuilder.toString(); | ||
if(!getFormula.equals(formula)){ | ||
output.errorMsg(Error.DOES_NOT_MATCH_DATA.getMsg()); | ||
} | ||
} |
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.
저도 굉장히 고민 많이 했던 것 같습니다. 저번에 save의 void에 대한 말씀을 해주셔서 해당하는 값을 return을 다른걸 주고 무엇을 할 수 있을까 생각하다가 넣은 값이랑 받은 값이 동일한지에 대한 체크를 해보았는데,, 지금 보니 당연히 동일한 것이 나올 수 밖에 없는거라고 생각이 드네요!! 오히려 코드만 길어지고 이것은 다시 고민해보고 다시 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.
리턴 값을 반드시 활용하지 않더라도 괜찮습니다! 그리고 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.
아 그렇네요! 인터페이스를 사용하면 리턴 값에 대해서 신중해야될 것 같습니다! 감사합니다!!! :)
private final FormulaSplitValidator formulaSplitValidator; | ||
|
||
public Validator() { | ||
this.output = Console.getInstance(); |
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.
Validator
에서 Output
객체를 사용해서 에러메시지를 출력할 필요가 있을까요..? boolean
으로 검증의 결과를 반환하니 메시지를 출력할지 말지는 호출한 측에서 처리하면 된다고 생각하는데 어떻게 생각하시나요..? 혹은 예외를 반환하는 것도 괜찮을 것 같습니다!
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.
말씀하신대로 boolean으로 검증의 결과를 Controller측에서 받을 수 있기 굳이 Validator에서 처리하지 않아도 될 것 같습니다! 예외 처리에 대한 부분들도 많이 안되어있어서 한 번 예외처리쪽으로 생각해보겠습니다! 감사합니다!
|
||
List<String> formularAfterSplitValidate = formulaSplitValidator.validate(formulaInput); | ||
|
||
if(formularAfterSplitValidate.size() != 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.
if(formularAfterSplitValidate.size() != 0) { | |
if(!formulaAfterSplitValidate.isEmpty()) { |
일관성이 있는 코드를 작성하게 되면 다른 사람들이 코드를 예측하기 쉬어지고 읽기 편해집니다~!
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.
헉! isEmpty메소드 있는것은 알았는데 전혀 쓸 생각을 못했던 것 같습니다! 각 객체에 대한 메소드도 한 번씩 다시 들여다보면서 활용할 수 있는 것들을 최대한 활용해보겠습니다!
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.
is
를 사용한 메서드가 많아보여서 해당 조건식도 일관되게 표현하시면 보기 더 좋을 것 같아보였습니다~!
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.
Deque, Queue, Stack과 같은 자료구조들은 is를 사용한 메서드가 익숙했는데 List여서 그런지 is메서드가 낯설게 느껴지네요ㅎㅎ 감사합니다!!
public Map<Long, String> getFormulaList() { | ||
return formulaMap; | ||
} |
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.
언급주신 방어적 복사에 대해 찾아보았습니다! 외부에서의 변경을 없애기 위해 방어적 복사를 통해 반환해보도록 하겠습니다!
StringBuilder stringBuilder = new StringBuilder(formula); | ||
stringBuilder.append(" = "); | ||
stringBuilder.append(caculateValue); |
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.
출력 포맷을 결정하는 것이 Repository의 책임이라고 볼 수 있을까요?
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.
출력 포맷을 정하는게 되게 단순한일이라고 생각이들어서 그냥 Repository의 책임이어도 될 것 같다고 생각하고 막 작성했던 것 같습니다! 사소한거라도 다시 한 번 책임에 대해서 고민하고 사용해보겠습니다!! :)
import java.util.Map; | ||
|
||
public interface Output { | ||
void menuMsg(); |
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 List<String> validate(String formulaInput){ | ||
formularAfterSplitValidate.clear(); | ||
String[] splitFormula = formulaInput.split(" "); | ||
|
||
for(String operatorOrOperand : splitFormula){ | ||
if(!splitValidateAddInList(operatorOrOperand)){ | ||
break; | ||
} | ||
} | ||
|
||
return formularAfterSplitValidate; | ||
} | ||
|
||
private boolean splitValidateAddInList(String operatorOrOperand){ | ||
if(Operator.isNumber(operatorOrOperand)) { | ||
formularAfterSplitValidate.add(operatorOrOperand); | ||
return true; | ||
} | ||
|
||
if(Operator.isSymbol(operatorOrOperand).isPresent()) { | ||
formularAfterSplitValidate.add(operatorOrOperand); | ||
return true; | ||
} | ||
|
||
if(!Operator.isNumber(operatorOrOperand) && !Operator.isSymbol(operatorOrOperand).isPresent()){ | ||
output.errorMsg(Error.CALCULATE_VALIDATION.getMsg()); | ||
formularAfterSplitValidate.clear(); | ||
return false; |
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 List<String> validate(String formulaInput){ | |
formularAfterSplitValidate.clear(); | |
String[] splitFormula = formulaInput.split(" "); | |
for(String operatorOrOperand : splitFormula){ | |
if(!splitValidateAddInList(operatorOrOperand)){ | |
break; | |
} | |
} | |
return formularAfterSplitValidate; | |
} | |
private boolean splitValidateAddInList(String operatorOrOperand){ | |
if(Operator.isNumber(operatorOrOperand)) { | |
formularAfterSplitValidate.add(operatorOrOperand); | |
return true; | |
} | |
if(Operator.isSymbol(operatorOrOperand).isPresent()) { | |
formularAfterSplitValidate.add(operatorOrOperand); | |
return true; | |
} | |
if(!Operator.isNumber(operatorOrOperand) && !Operator.isSymbol(operatorOrOperand).isPresent()){ | |
output.errorMsg(Error.CALCULATE_VALIDATION.getMsg()); | |
formularAfterSplitValidate.clear(); | |
return false; | |
public List<String> validate(String formulaInput) { | |
formularAfterSplitValidate.clear(); | |
String[] splitFormula = formulaInput.split(" "); | |
for (String operatorOrOperand : splitFormula) { | |
if (!isValid(operatorOrOperand)) { | |
throw new IllegalArgumentException(Error.CALCULATE_VALIDATION.getMsg()); | |
} | |
formularAfterSplitValidate.add(operatorOrOperand); | |
} | |
return formularAfterSplitValidate; | |
} | |
private boolean isValid(String operatorOrOperand) { | |
return Operator.isNumber(operatorOrOperand) || | |
Operator.isSymbol(operatorOrOperand).isPresent(); | |
} |
불필요한 연산과 중복된 코드를 줄여서 메서드가 하나의 기능을 담당하도록 하면 좋을 것 같습니다!!
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 FormulaRepository formulaRepository; | ||
private final Validator validator; | ||
|
||
Calculator(Calculation calculation, FormulaRepository formulaRepository){ |
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.
접근제어자를 default
로 하신 이유가 있나요?
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.
다른 기능들의 리팩토링에 집중하다가 접근제어자가 default로 되어있던 것을 미처 확인하지 못했던 것 같습니다!! 필드가 아닌 생성자 메소드여서 오히려 더 잘 안보였던 것 같아요. 별 다른 이유는 없어서 지정하는 쪽으로 수정해보겠습니다!
return isSymbol(symbol).get().expression.apply(firstOperand, secondOperand); | ||
} | ||
|
||
public static Optional<Operator> isSymbol(String inputSymbol){ |
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.
메서드 명에 반드시 지켜야 하는 규칙은 없습니다만, 일반적으로 자주 사용되는 관용적인 표현이 있습니다!
'is'
의 경우, 맞는지 틀린지 판단하는 메서드 명에 자주 사용됩니다.
그렇다면 일반적으로 isSymbol
를 보고 어떤 메서드일 것이라고 생각할 수 있을까요?
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.
메서드 네이밍이 가장 어려운 것 같습니다. isSymbol이라고 하면 아마 해당 Symbol을 찾아서 반환한다기보다는 해당 Symbol이 맞는지 틀린지에 대한 결과값이 나오는 것이 맞다고 생각이 듭니다! 말씀해주신 관용적인 표현을 토대로 변경해보도록 하겠습니다!
} | ||
|
||
public static Optional<Operator> isSymbol(String inputSymbol){ | ||
return Arrays.stream(values()) |
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.
네! 한 번 최적화에 대해서도 고려해보고 반영해보겠습니다! :)
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.
@Hchanghyeon
안녕하세요. 창현님 리팩터링이 끝난 것 같아서 마지막 리뷰만 남기고 해당 PR은 머지하도록 하겠습니다!
리뷰 내용은 반영하시지 않아도 됩니다. 한번 확인 해보시고 다음 과제에서 생각해보시면 좋을 것 같습니다!! 😄
과제하시느라 정말 고생많으셨습니다! 실력이 정말 빠르게 늘어가는 것 같습니다~~ 👍👍 화이팅! 💪
formulaRepository.save(formulaAndResult); | ||
} | ||
|
||
private String formattingFormula(String inputFormula, String 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.
만약 저였다면 inputFormula와 result를 묶는 데이터 객체(DTO)를 만들고 toString 메서드를 포맷에 맞게 재정의해서 사용했을 것 같아요!
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.
DTO에 대한 고민을 전혀 하지 못했던 것 같습니다! 고민을 해보면 생각보다 DTO가 쓰일 수 있는 부분이 많은 것 같더라구요! 한 번 사용해서 수정하겠습니다!
|
||
public interface FormulaRepository { | ||
void save(String formulaAndResult); | ||
Map<Long, String> getFormulaList(); |
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.
getFormulaList
메서드 명과 달리 반환타입은 Map
이네요!
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 int getPriority() { |
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.
IntelliJ 자동 정렬 기능을 이용하시면 좋을 것 같아요!
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.
이것을 리팩토링이 다 끝나고 ㅠㅠ 알게되었네요 감사합니다!! 추가 해놓겠습니다!!
// Arrays.stream(values()).filter(operator -> operator.symbol.equals(inputSymbol)).findAny(); | ||
} | ||
|
||
private String getSymbol() { |
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
으로 두신 이유가 궁금합니다!
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으로 두었습니다!
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.
외부에서 사용하지 않는다면 getter
가 아닌 필드에서 접근해도 괜찮지 않을까요?
} | ||
|
||
public static boolean isNumber(String inputNumber){ | ||
return Pattern.matches("[0-9]+",inputNumber); |
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.
Pattern 객체가 생성비용이 크다는 사실 알고 계신가요? 꼭 그러한 상황이 아니더라도 상수화할 수 있는 부분이 있다면 상수화 혹은 캐싱해두는게 좋습니다!
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.
String.matchs를 Pattern 객체로 바꿔서 사용하라 라는말만 들었는데 말씀해주신 링크 들어가보니 객체 생성비용이 정말 크네요,,! 앞으로 이러한 반복 객체들을 상수화 시켜볼 수 있도록 하겠습니다!
|
||
for (String operatorOrOperand : splitFormula) { | ||
if (!isValid(operatorOrOperand)) { | ||
throw new IllegalArgumentException(Error.CALCULATE_VALIDATION.getMsg()); |
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.
아! 사실 예외 잡는 것에 대한 개념이 좀 부족했던 것 같습니다! 어제 종운님이랑 얘기하면서 예외 잡는 것에 대해서 얘기했었는데 이제야 막 적용할 수 있을 것 같아요!! 다음번 과제에서는 예외날 수 있는 곳에서 잡는거 한 번 해보도록 하겠습니다!!
static Stream<Arguments> makePostfixList() { | ||
return Stream.of( | ||
Arguments.of(Arrays.asList("3", "5", "10", "*", "+"), "53"), | ||
Arguments.of(Arrays.asList("3", "5", "10", "-", "+"), "-2"), | ||
Arguments.of(Arrays.asList("3", "10", "5", "/", "+"), "5") | ||
); | ||
} | ||
|
||
@Test | ||
@DisplayName("0으로 나눌 때 예외가 발생") | ||
void checkZeroDivide() { |
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.
테스트 케이스를 짜는 것이 생각보다 많이 어려운 것 같아요. 오히려 기능을 구현하는 것보다 테스트 케이스를 생각해내는 것이 리소스가 더 많이 드는 느낌이라고 해야할까요? 말씀해주신 엣지케이스랑 다른 여러 케이스들에 대한 개념들도 한 번 공부해서 적용시켜보겠습니다!!
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.
주성 서브멘토님! 2주동안 정말 많은 리뷰달아주셔서 짧은 기간동안 많은 것을 배울 수 있었던 것 같습니다! 앞으로 코드를 작성할 때는 멘토님께서 말씀하신 내용들을 최대한 반영해서 개선해볼 수 있도록 할게요! 너무너무 감사드립니다 :)
} | ||
private int getPriority() { |
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.
이것을 리팩토링이 다 끝나고 ㅠㅠ 알게되었네요 감사합니다!! 추가 해놓겠습니다!!
formulaRepository.save(formulaAndResult); | ||
} | ||
|
||
private String formattingFormula(String inputFormula, String 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.
DTO에 대한 고민을 전혀 하지 못했던 것 같습니다! 고민을 해보면 생각보다 DTO가 쓰일 수 있는 부분이 많은 것 같더라구요! 한 번 사용해서 수정하겠습니다!
// Arrays.stream(values()).filter(operator -> operator.symbol.equals(inputSymbol)).findAny(); | ||
} | ||
|
||
private String getSymbol() { |
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으로 두었습니다!
|
||
for (String operatorOrOperand : splitFormula) { | ||
if (!isValid(operatorOrOperand)) { | ||
throw new IllegalArgumentException(Error.CALCULATE_VALIDATION.getMsg()); |
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 interface FormulaRepository { | ||
void save(String formulaAndResult); | ||
Map<Long, String> getFormulaList(); |
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 static boolean isNumber(String inputNumber){ | ||
return Pattern.matches("[0-9]+",inputNumber); |
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.
String.matchs를 Pattern 객체로 바꿔서 사용하라 라는말만 들었는데 말씀해주신 링크 들어가보니 객체 생성비용이 정말 크네요,,! 앞으로 이러한 반복 객체들을 상수화 시켜볼 수 있도록 하겠습니다!
static Stream<Arguments> makePostfixList() { | ||
return Stream.of( | ||
Arguments.of(Arrays.asList("3", "5", "10", "*", "+"), "53"), | ||
Arguments.of(Arrays.asList("3", "5", "10", "-", "+"), "-2"), | ||
Arguments.of(Arrays.asList("3", "10", "5", "/", "+"), "5") | ||
); | ||
} | ||
|
||
@Test | ||
@DisplayName("0으로 나눌 때 예외가 발생") | ||
void checkZeroDivide() { |
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.
테스트 케이스를 짜는 것이 생각보다 많이 어려운 것 같아요. 오히려 기능을 구현하는 것보다 테스트 케이스를 생각해내는 것이 리소스가 더 많이 드는 느낌이라고 해야할까요? 말씀해주신 엣지케이스랑 다른 여러 케이스들에 대한 개념들도 한 번 공부해서 적용시켜보겠습니다!!
📌 과제 설명
TDD와 OOP를 활용하여 계산기 만들기.
👩💻 요구 사항과 구현 내용
콘솔
검증
연산
저장소
실행결과
✅ 피드백 반영사항
✅ PR 포인트 & 궁금한 점
자바와 객체지향에 대해서 공부한지 오래되지 않아 많이 부족하지만 최대한 각 객체의 책임을 구분하려 했습니다! 그리고 아직 객체지향에 대한 개념이 부족하여 구현에 초점을 맞춰서 작성했습니다! 감안해서 봐주시면 감사하겠습니다!! 😊
→ 저의 경우 원하는 값이 아닐 경우 따로 에러를 throw하지 않고 잘못 입력되었다고 안내해주고 다시 입력 값을 받을 수 있도록 구현했습니다. 이것은 본인이 생각한 로직에 맞게끔 작성하면 되는걸까요?