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

Open
wants to merge 17 commits into
base: bjo6300
Choose a base branch
from

Conversation

bjo6300
Copy link
Member

@bjo6300 bjo6300 commented Jun 14, 2023

📌 과제 설명

  • 객체지향적인 코드로 계산기 구현하기
    • 더하기
    • 빼기
    • 곱하기
    • 나누기
    • 우선순위(사칙연산)
  • 테스트 코드 구현하기
  • 계산 이력을 맵으로 데이터 저장기능 만들기
    • 애플리케이션이 동작하는 동안 데이터베이스 외에 데이터를 저장할 수 있는 방법을 고안해보세요.

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

  • feat : I/O 구현, Menu 구성
    전체적인 흐름을 파악하고 틀을 잡았습니다.

  • feat : 계산식 입력 받기
    Input, Output 인터페이스를 만들고 Menu 클래스를 만들어 Input, Output을 상속받았습니다. 상속받은 인터페이스를 통해 계산식을 입력받고 공백을 제거했습니다.

  • feat : 중위 표기식을 후위 표기식으로 변환
    입력받은 중위 표기식을 후위 표기식으로 변환하는 함수를 추가했습니다.

  • feat : 후위 표기식 계산 및 출력
    변환된 후위 표기식을 계산하고 결과를 출력했습니다.

  • feat : 조회 기능 구현
    Map<Integer, String>을 구현해서 인덱스를 키 값, 입력받은 수식을 value로 지정했습니다. 그리고 이 Map에 데이터를 저장하고 조회하는 기능을 구현했습니다.

  • style : 코드 컨벤션 정리
    이해하기 수월하게 함수 명과 변수 명을 수정했습니다.

  • feat : 함수 모듈화
    while문의 조건이 길어서 모듈화 시켰습니다.

  • feat : 명확한 case 분리
    변수 할당으로 이름 지어서 가독성을 향상시켰습니다.

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

  • 구글 자바 포멧을 IntelliJ에 적용시켜 코드 컨벤션을 통일시켰습니다.
    (테스트 코드는 마감 기한 내에 빠르게 작성하겠습니다..!)
  • 객체지향적인 설계에 관한 피드백을 받고 싶습니다!
  • 역할과 책임이 올바르게 나누어져 있는지 궁금합니다.

@bjo6300 bjo6300 self-assigned this Jun 14, 2023
@bjo6300 bjo6300 changed the base branch from Junil to bjo6300 June 14, 2023 21:01
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.

  • 객체지향적 설계는 함부로 말씀드리긴 힘들지만 History를 인터페이스로 짠다던가, Menu를 분리한다던가, 있을 수 있겠네요. 또 showHistory도 결국 출력인데 output을 하는곳에서는 모두 output으로 관리해서 di를 받는것도 괜찮아보이구요!
  • 역할과 책임 부분은 제가 말한것 좀 더 생각하시면 될것같습니다. 제가 뭔가 말씀드리기 애매하네여..

Comment on lines +7 to +17
private static int getPriority(char operator) {
switch (operator) {
case '+':
case '-':
return 1;
case '*':
case '/':
return 2;
}
return -1;
}
Copy link

Choose a reason for hiding this comment

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

우선순위는 Operator Enum에 있어도 될것같아여, 내부 변수라던가 활용하면 좋겠네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 의견 감사합니다!

import com.programmers.engine.io.Output;
import java.util.Scanner;

public class Menu implements Input, Output {
Copy link

Choose a reason for hiding this comment

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

Menu를 다중 상속으로 구현체로 쓴다면, 나중에 Input이나 Output중에 각각 따로 바뀌는경우에는 따로 빼시는건가요? 예를들면 Output은 file로 출력하거나 하는 경우가 있겠네요.

Copy link
Member Author

Choose a reason for hiding this comment

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

메뉴가 두 가지 책임을 가지고 있어보이네요..! 고민해서 분리해보겠습니다.

Comment on lines +15 to +18
private final Input input;
private final Output output;
private final CalculationFormula calculationFormula;
private final Menu menu;
Copy link

Choose a reason for hiding this comment

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

interface를 썼는데 굳이 Menu를 또 받는 이유가 궁금합니다.


import java.util.Stack;

public class InfixToPostfix {
Copy link

Choose a reason for hiding this comment

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

다 static을 쓴 이유가 따로 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

다시 생각해보니 static을 사용할 이유가 없어보입니다. 수정하겠습니다!

Comment on lines +14 to +24
if (ch == ' ') {
continue;
}

if (isDigit(ch)) {
operandStack.push((double) (ch - '0'));
} else {
Operator operator = Operator.fromSymbol(ch);
double rightOperand = operandStack.pop();
double leftOperand = operandStack.pop();
double result = operator.calculate(leftOperand, rightOperand);
Copy link

Choose a reason for hiding this comment

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

ch를 통해서 validation 했다면 switch도 좋아보이네요.� 하지만 수정은 하실필요없습니다. switch쓰는게 java에서는 안쓰는곳도 많은걸로알고있어서..

@joseph-100
Copy link

고생하셨습니다.
훈기님 리뷰에 덧붙일게요.

  • Calculator 생성시 menu를 3개 넘기는건 매우 부 자연스럽습니다. 메뉴가 IO 를 구현하는게 아니라 멤버로 가지고 있어야 더 자연스럽지 않을지 생각이 드네요.
  • CalculationFormula 에서 멤버필드로 menu를 또 생성하는데 메인메소드에서 생성한걸 주입받아 사용할 순 없을까요?
  • CalculationFormula calculate 메소드에서 history.save() 는 부가기능으로 보입니다. 호출한 상위 계층에서 다뤄야할 내용 같아요.

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.

3 participants