Skip to content
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주차 과제 : 계산기 구현 미션 제출합니다. #161

Open
wants to merge 21 commits into
base: JuwoongKim
Choose a base branch
from

Conversation

JuwoongKim
Copy link
Member

📌 과제 설명

  • TDD와 OOP를 활용한 계산기 구현

👩‍💻 요구 사항과 구현 내용

기능목록

  • 콘솔 기능
  • 계산 기능
    • 사칙연산
    • 다항식 연산 (연산자 우선순위 고려)
  • 테스트 코드 작성
  • 데이터 저장 기능(Map 사용)

기타


✅ PR 포인트 & 궁금한 점

  • 계산기, 콘솔, 저장소, 객체의 협력을 어떻게 구성하는 것이 객체지향적일까요?

    • 계산기 객체가 콘솔, 저장소를 생성자로 주입빋아 사용하는 것으로 구현했습니다

    • 사실 콘솔의 필요성을 느끼지 못하고, 계산기 메서드 자체에서 JAVA/IO 패키기를 사용하면 어떨까 생각합니다.

      → 표준입출력이 아닐 수 있으니input, output 인터페이스를 만들고 구현체를 만들어야하나?

  • 예외처리

    • 어떤 상황에서 모아서 처리하고, 어떤 상황에서 개별적으로 처리하는 것이 좋은지, 구체적인사례를 바탕으로 이해하고 싶습니다.
    • 예외처리를 전부 에러메시지 출력 + continue로 했습니다. 적절한 방법일까요?
  • 외부에서 관계맺기

    • 스프링컨테이너가 빈들을 생성하고 서로의 의존성을 맺어주는 것처럼 수정을 해보고 싶습니다.
    • 이유는 콘솔, 저장소를 인터페이스로 구현하고, 필요할 때마다 구현체를 교체하며 (나름? 결합도를 낮춰보고 싶습니다)
    • 약간 오바하는 느낌이라, 시기상조인지, 불필요한지, 정도에 대한 피드백을 받고 싶습니다.

* Intellij를 사용해 Java Project 생성 
* gitignore.io 를 통해 불필요한 파일 숨김 ( Intellij+all , gradle, java)
*  기존 상황에서 org.junit.jupiter.params를 인식하지 못해 @ParameterizedTest를 사용할 수 없었음

todo : 정확한 원인을 확인해야함
* 에너테이션 사용을 위한 롬북 추가
* Calulator clas 에서  lombok으로 @AllArg... 생성하려 했으나, 오류남 
 -> 우선 코드로 직접 생성자를 작성하고 추후 변경 예정
* validateExpression 메서드를 통해 구현함 
* 함수 내부의 정규표현식을 사용해 검증
* trim 처리는 외부의 compute 메서드에서 진행 (다시 parsing해야하니깐)
* trim을 사용해 앞, 뒤 공백 제거
* 정규표현식을 사용해, 문자열 내부 공백을 한칸으로, 숫자사이는 공백없음으로 전처리
* 불변객체인 문자열을 계속 입력값으로 사용하는 것이 마음에 걸림 -> 메모리 낭비
* 문자열 배열로 변경
* 기존 배열 타입에서 크기 변경에 자유로운 리스트 형태로 변환 
* 후위 표기식 변환 시 연산자 우선순위를 위해  사칙연산에 대한 enum 을 추가함
* 후위식 표현식 리스트를 인자로 받아 우선순위에 맞게 사칙연산 구현 

todo: 0으로 나눌 시 ArithmeticException 를 던지도록 하였지만, 처리부분을 작성하지 않음
* 예외 처리는 다른 부분과 함께 처리할 예정  ... 처리 범위 생각중
* 프로그램 수행 시  계산을 3번 정도 반복하다 보면, 입력을 받으면서   
"Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0."  waring
이 발생하며 종료됨

* 버전을 변경했지만, 9.0을 가리킴,  조사를 통해 intellij setting을 gradle(default)가 아닌 idea로 변경함
todo : 구현 완료 후 원인을 정확히 정리해야 함
* 추가적인 문제로 Scanner 사용 시, 모드 선택 값을 받고  연산입력값을 받을 떄, 입력 값을 받지 않고, 그냥 넘어감 
* gradle-wrapper버전변경, BufferedReader 사용 동  이것저것 변경하여 문제 현상이 발생하지 않지만, 원인을 다시 확인해야함
* 더하기에 대한 함수를 복사하고 수정하지 않음
* 예외를 처리하는 범위를  Caculator 실행 전체 범위로 잡음
* 한 곳에서 예외를 처리해  보기가 쉽지만, 추후에 분리하여 처리해야함
* 네이버 캠퍼스 핵데이 Java 코딩 컨벤션을 따름 
* https://naver.github.io/hackday-conventions-java/

* 인텔리제이에 해당 fomatter 적용
* https://bestinu.tistory.com/64
* 계산 순서를 보장한다는 것을 전달하고 싶어 Ordered 라는 명칭을 앞에 추가함
* 에러 메시지만 출력함 ( 에러코드 없음 )
Copy link

@Hunkik Hunkik left a comment

Choose a reason for hiding this comment

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

계산기 과제 고생하셨습니다 주웅님! 질문하신것에 대한 답변은 참고만 해주시면 좋을것같아여.(정답이 아닙니다) 시간이 많이 없어서.. 간단하게 리뷰했습니다.

  1. 말씀하신것처럼 저도 한다면 말씀하신것처럼 구현할 것 같습니다. 표준입출력이 아닐 수 있으니 input,output 인터페이스를 만들고 구현체를 만들어 main에서 DI를 통해 제어할 것 같네요!
  2. 예외처리 같은 경우는 저도 아직 어떤게 더 좋다고 말씀은 못드리겠네요.. 현재 예외처리에 대한 요구사항이 없으니 괜찮아보입니다.
  3. 현재 과제에서는 좋은 생각이라고 생각합니다! 하지만 지금 코드에서 꽤 많이 수정해야겠네요.. 화이팅입니다

System.out.println("1. 조회");
System.out.println("2. 계산");
String inputSelectMode = br.readLine();
if (!(inputSelectMode.equals("1") || inputSelectMode.equals("2"))) {
Copy link

Choose a reason for hiding this comment

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

이 부분은 확실한 표현을 위해 매직넘버 해결해주세요!

Comment on lines +29 to +31

while (true) {

Copy link

Choose a reason for hiding this comment

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

다른곳은 메서드 시작시에 공백줄이 없는데 여기엔 있네요. 혹시 무슨 의미가 있을까요?

Comment on lines +33 to +40
int mode = console.inputSelectMode();
console.outputSelectResult(mode);
switch (mode) {
case 1:
loadHistory();
break;
case 2:
compute();
Copy link

Choose a reason for hiding this comment

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

여기도 mode가 그냥 int보단 의미가 있는 상수나 열거형을 쓰면 더 좋아보입니다

Comment on lines +84 to +85
String regex = "^\\d+(\\s*[+\\-*/]\\s*\\d+)*$";
return Pattern.matches(regex, expression);
Copy link

Choose a reason for hiding this comment

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

정규표현식을 매번 matchers를 사용하면 매번 패턴을 생성해서 비용이 비싼걸로 알고 있습니다! 컴파일을 하여 패턴을 컴파일타임에 만든 뒤 재사용하면 비용적으로 더 좋다고 생각합니다


public List<String> infixToPostfix(List<String> infixExpression) {
List<String> postfixExpression = new ArrayList<>();
Stack<String> st = new Stack<>();
Copy link

Choose a reason for hiding this comment

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

st가 수식 계산을 하기 위한 스택인가요? 의미있는 이름이면 더 좋다고 생각합니다

Comment on lines +117 to +118
while (!st.isEmpty())
postfixExpression.add(st.pop());
Copy link

Choose a reason for hiding this comment

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

한 줄 이여도 중괄호가 있는게 구분감이 생겨 가독성이 더 좋다고 생각합니다! 몰론 팀 컨벤션마다 다를 수 있다고 생각합니다..

@joseph-100
Copy link

고생하셨습니다.
훈기님 리뷰 외에 추가 내용만 드릴게요.

  • 프로그램을 정상종료하는 부분이 있어야 합니다. Calculator run() 메소드에 while문을 나갈 정상 루트를 만들어 주세요.
  • catch 한곳에 continue 가 필요할까요?
  • Calculator compute() 메소드에 커스텀 런타임 익셉션을 명시하셨는데, 런타임은 명시할 필요 없습니다. 자바의 Checked, Unchecked 익셉션에 대해 조금 더 찾아보시길 바랍니다.
  • Calculator 가 가지는 책임이 너무 많은것 같아요. 지금 Calculator는 컨트롤러같이 입력을 구분하게하고, 실제 계산로직은 다른 클래스로 나누는게 좋을 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants