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

Open
wants to merge 18 commits into
base: hyeon-z
Choose a base branch
from

Conversation

hyeon-z
Copy link

@hyeon-z hyeon-z commented Jun 14, 2023

📌 과제 설명

요구사항

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

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

  • 각 연산자 interface 구현
    더하기, 빼기, 곱하기, 나누기를 interface로 구현하여 operator package에 포함했습니다.

  • 연산자 클래스 구현
    각 연산자 interface를 구현하는 연산자 클래스를 operator package에 포함했습니다.

  • 선택 항목 출력하는 Choice 클래스 구현
    콘솔에 선택 항목을 출력하넌 Choice클래스를 구현했습니다.

  • 계산기 기능 - 계산 구현
    계산기 기능 중 하나인 계산을 담당하는 클래스를 구현하였습니다.
    중위표현식을 후위 표현식으로 변환하는 메서드와 후위 표현식으로 결과를 구하는 메서드로 이루어져 있습니다.

  • 계산기 기능 - 저장 구현
    Map을 통해 계산기 계산 기록을 저장하는 클래스를 구현하였습니다.
    계산 기록을 저장하는 메서드와 전체 계산 이력을 출력하는 메서드로 이루어져있습니다.

  • 계산기 클래스 구현
    반복하며 계산기를 동작시키는 클래스를 구현하였습니다.

  • Main함수 구현
    계산기 객체를 생성하여 동작시켰습니다.

  • 테스트 코드

    • Validator메서드를 테스트 하기
    • 계산기 동작을 테스트 하기
      두 가지 종류의 테스트 코드로 구현했습니다.

전체구조
Add, Subtract, Divide, Multiply interface를 구현하는 Operator클래스
생성자로 Operator를 초기화하는 Caculation(계산)클래스
생성자로 Caculation, Storage를 초기화하는 Calculator(계산기)클래스

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

  • 객체 지향적으로 짜기 위해 노력했는데 어쩔 수 없이 한 메서드에 넣는 코드가 길어지는 경우가 있어서 어떤 방식으로 구현하면 좋은지 궁금합니다.
  • package분리를 같은 기능을 가지는 하려고 했는데 잘 분리된 것인지 궁금합니다.
  • OOP 설계에 관한 피드백을 받고 싶습니다! (객체 지향적으로 잘 짠 것인지)
  • 테스트 코드를 처음 구현해보는데 이러한 방식이 맞는 지 궁금합니다.
  • 테스트 코드에서는 상수나 클래스가 변경되지 않아서 final을 적극적으로 사용했는데 해당 방식이 맞는 지 궁금합니다.
  • Validate 기능을 가지는 메서드들을 Validator 클래스로 분리하여 구현헀는데, 공통적으로 여러 군데에서 쓰이지 않는 경우에는 해당 메서드가 쓰이는 클래스에 구현하는 것이 좋은지 이처름 Validator로 분리하는 것이 좋은지 궁금합니다.

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.

계산기 과제 고생하셨습니다. 코멘트 참고해주세요

  • 메서드에 코드가 길어진다면, 길어진 부분을 기능적으로 하나로 묶어서 다시 메서드로 만들면 됩니다! 재사용성이 없다면, private로 내부에, 아니라면 더 고려해서 짜시면 될 것 같아여
  • 객체지향적으로 짰다고 하기는 조금 애매한것같습니다. 이후에 확장이나 재사용성, 협력, SOLID원칙 등을 봤을때 지켜지지 않는 부분이 많아서 이를 고려해서 생각해보시면 좋을것같아여. 예를들면 main에서 Input,Output,Storage를 DI를 통해 추후 다른 Input으로 구현체가 변경되어도 서비스로직은 그대로 유지한다던가 하는 방식이 있곘네요
  • 테스트 코드에서는 굳이 안쓰셔도 될것같아여! 최대한 다양한 케이스를 테스트 한다고 생각하는게 더 중요하다고 생각합니다.
  • 굳이 많이 안쓰인다면 validation 쓰는곳 바로 아래에 private 메서드로 쓰시는게 더 좋아보입니다.

int choice = checkChoiceNum(Integer.parseInt(br.readLine()));
System.out.println();

if (choice == 0) {
Copy link

Choose a reason for hiding this comment

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

매직넘버보단 의미있는 상수, 열거체 등을 사용해주세요!


br = new BufferedReader(new InputStreamReader(System.in));
int choice = checkChoiceNum(Integer.parseInt(br.readLine()));
System.out.println();
Copy link

Choose a reason for hiding this comment

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

굳이 필요할까 싶습니다! checkChoiceNum 이후 필요한거면 거기에 넣어도 될것같아여

Comment on lines +37 to +38
br = new BufferedReader(new InputStreamReader(System.in));
String expression = br.readLine();
Copy link

Choose a reason for hiding this comment

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

버퍼리더 close하는곳이 없네여! 버퍼는 쓰고 닫는 습관이 있으면 좋다고 생각합니다. 리더를 열고 닫는 곳은 선택에 맡기겠습니다 ㅎㅎ

import static validator.Validator.checkValidOperator;

public class Calculation {
Operator operator;
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 +34
Stack<Character> stack = new Stack<>();
StringBuilder sb = new StringBuilder();

String[] split = expression.split(" ");

for (String now : split) {
Copy link

Choose a reason for hiding this comment

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

stack,sb,split,now등 의미를 알기가 힘드네요!


char c = checkValidOperator(now.charAt(0));

switch (c) {
Copy link

Choose a reason for hiding this comment

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

매직넘버라 바꿔주세요!

}

private static boolean isDigit(String str) {
return str.matches(positiveRegularExp);
Copy link

Choose a reason for hiding this comment

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

Pattern 컴파일 후 사용하면 더 성능적으로 좋습니다

@@ -0,0 +1,24 @@
package operator;

public class Operator implements Add, Subtract, Divide, Multiply {
Copy link

Choose a reason for hiding this comment

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

Add, Subtract, Divide, Multiply같은경우에 interface로 만든 이유가 있나요?

Comment on lines +3 to +11
public class Choice {
public static void printChoice() {
System.out.println("0. 종료");
System.out.println("1. 조회");
System.out.println("2. 계산");

System.out.println();
System.out.print("선택: ");
}
Copy link

Choose a reason for hiding this comment

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

프린트로 패키지를 나누셨는데 Choice만 있는게 뭔가 어색해보이네요! 결과 같은경우도 print할 수 있고, 다양한 경우가 있는데 Choice만 있는 이유가 있을까요?

@joseph-100
Copy link

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

  • Calculator 의 run() 메소드에서 IO 관련 코드들이 같이 있어 코드를 읽는데 방해가 됩니다. IO 관련 책임은 다른 객체에서 하는게 좋을 것 같아요.
  • Validator에서 익셉션을 던지고 있는데 적절히 핸들링이 필요할 것 같아요. 입력을 잘못받았을땐 안내와 함께 재입력을 받는게 좋을 것 같습니다. 자바의 Checked, Unchecked 익셉션에 대해서도 찾아보시기를 추천합니다.

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