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

[계산기] 권민관 미션 1차 수정본 입니다! #25

Open
wants to merge 40 commits into
base: mlngwan
Choose a base branch
from

Conversation

mlngwan
Copy link

@mlngwan mlngwan commented May 11, 2024

초간단 계산기와 문자열 계산기 구현해보았습니다.
테스트 코드를 처음 공부해보는데 덕분에 좋은 공부했습니다. 감사합니다!

Copy link

@sosow0212 sosow0212 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!
미션 요구사항이 지켜지지 않은 것 같은데 한 번 확인 부탁드릴게요
정렬 습관화 들이면 더 좋을 것 같습니다!

import simpleCalculator.view.CalculatorView;
import java.util.List;

public class CalculatorController {

Choose a reason for hiding this comment

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

해당 클래스 외에도 첫줄 개행 추가해주세요

Copy link
Author

@mlngwan mlngwan May 15, 2024

Choose a reason for hiding this comment

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

네 수정하겠습니다!

Comment on lines 10 to 28
public void doAdd() {
int addResult = calculator.addNumbers();
System.out.println("덧셈 결과 : " + addResult);
}

public void doSub(){
int subResult = calculator.subNumbers();
System.out.println("뺄셈 결과 : " + subResult);
}

public void doDivide(){
int divideResult = calculator.divideNumbers();
System.out.println("나눗셈 결과 : " + divideResult);
}

public void doMultiple(){
int multipleResult = calculator.multipleNumbers();
System.out.println("곱셈 결과 : " + multipleResult);
}

Choose a reason for hiding this comment

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

사용자의 입력에 따라서 곱셈인지 뺄셈인지 등등 나뉘어야할 것 같아요
지금은 모든 결과를 출력하는데 다른 방식으로 해야할 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

따로 결과를 출력하는 OutputView로 분리하여 출력하도록 수정하겠습니다.

private final int num1;
private final int num2;

public Calculator(int num1, int num2){

Choose a reason for hiding this comment

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

음수일 때 예외가 없습니다 요구사항 지켜주세요~

Copy link
Author

Choose a reason for hiding this comment

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

음수일 때 예외처리 하도록 수정했습니다.

return num1 - num2;
}

public int divideNumbers(){

Choose a reason for hiding this comment

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

  1. 정렬이 안 되어있습니다
  2. 나누는 수가 0일 때 예외가 없습니다

Copy link
Author

Choose a reason for hiding this comment

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

  1. 모든 코드들 정렬했습니다!
  2. 나누는 수가 0일 때 예외 처리 진행했습니다!

public int multipleNumbers(){
return num1 * num2;
}
}

Choose a reason for hiding this comment

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

EOF가 발생하네요 검색하셔서 해결해주세요~

Copy link
Author

Choose a reason for hiding this comment

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

EOF에 대해 맞게 이해 했는지와 궁금한 점이 생겼습니다.
입력의 개수가 정해지지 않은 경우에만 더 이상 입력이 없게 하기 위해 사용하는 것으로 이해했습니다.
scanner.close()와 차이점이 무엇인가요??

Choose a reason for hiding this comment

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

scanner.close()는 resource를 풀어주는 거라 다른 결이긴 합니다!
해당 링크 참고해보시면 좋을 것 같아요!

return number;
}

public int sum(int[] inputList){

Choose a reason for hiding this comment

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

sum이 여기도 있는 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

메서드 네이밍이 헷갈려서 그런가요??
아니면 필요 없는 메서드라고 생각하셔서 그러신 걸까요??
해당 메서드는 배열 안의 요소를 합하는 메서드라 필요하다고 생각했습니다!
혹시 메서드 명이 문제라면 수정하도록 하겠습니다.

Comment on lines 40 to 42
int testNum1 = 2;
int testNum2 = 1;
Calculator calculator = new Calculator(testNum1, testNum2);

Choose a reason for hiding this comment

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

위 아래로 계행 하나씩 추가해주세요

Copy link
Author

Choose a reason for hiding this comment

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

넵 수정했습니다!

void addTest(){
//given
int expectValue = 3;
int testAdd = calculator.addNumbers();

Choose a reason for hiding this comment

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

이건 when으로 가야겠네요~

Copy link
Author

Choose a reason for hiding this comment

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

넵 확인하고 수정 완료 했습니다!

int testNum1 = 2;
int testNum2 = 1;
Calculator calculator = new Calculator(testNum1, testNum2);
@DisplayName("더하기 테스트 1")

Choose a reason for hiding this comment

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

예외를 검증하는 거면 네이밍을 바꾸는게 좋을 것 같습니다

Copy link
Author

Choose a reason for hiding this comment

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

예외 테스트 네이밍 명시적으로 수정 완료했습니다.

@@ -0,0 +1,57 @@
package stringCalcculator.model;

Choose a reason for hiding this comment

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

패키지 명 오타났네요!
그리고 다른 패키지에 둔 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

다른 패키지에 둘 이유가 없다고 생각해서 기존의 model 패키지에 합병했습니다!

@mlngwan mlngwan changed the title [계산기] 권민관 미션 제출입니다! [계산기] 권민관 미션 1차 수정본 입니다! May 16, 2024
@mlngwan
Copy link
Author

mlngwan commented May 16, 2024

코드 전체적으로 Refactoring 진행하였습니다.
진행 내역은 다음과 같습니다.

  • 코드 전반적으로 내부에서 사용하는 메서드는 public이 아닌 private를 사용하도록 접근제어자 수정했습니다.

  • EOF를 처리하도록 수정했습니다.

  • 의미없는 static을 사용하지 않도록 수정했습니다.

  • Code Convention

    • Class 첫 줄에는 개행이 있도록 추가했습니다.
    • 메서드 명은 동사로 시작하도록 수정했습니다.
    • 메서드 명시 다음의 띄어쓰기 하도록 수정했습니다.
    • 자주 사용하는 값들은 상수로 사용하게 수정했습니다.
    • 상수 네이밍은 좀 더 명시적으로 수정했습니다.
  • Model Refactoring

    • Calculator가 요구 사항을 지키도록 수정했습니다. (0으로 나누기가 불가, 음수 예외 추가)
    • 추가적으로 입력 인자가 2개인지 검사하도록 수정했습니다.
    • 기존의 Model에서 View에서 처리할 부분이 있었습니다. View에서 처리하도록 수정했습니다.
    • StringCalculator 구분하지 않도록 수정했습니다.
  • View Refactoring

    • View를 InputView와 OutputView로 분리하였습니다.
    • 기존의 View에서 Model이 처리할 부분이 있었습니다. Model에서 처리하도록 수정했습니다.
  • Controller Refactoring

    • Model과 View 수정에 맞춰 Controller도 잘 작동하도록 수정했습니다.
    • Controller에서 InputView에서 받아온 값들을 Model의 로직으로 처리하고 결과를 OutputView로 반환하도록 수정했습니다.
  • Test Refactoring

    • 기존의 코드들의 수정에 따라 Test가 잘 작동하게 수정했습니다.
    • 보기 편하도록 Test하는 방법이 비슷하면 같은 분류로 묶었습니다.

Copy link

@sosow0212 sosow0212 left a comment

Choose a reason for hiding this comment

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

이번 단계는 여기서 마무리하겠습니다!
변수명과 기본적인 자바에 대해서 리뷰를 했는데, 리뷰 내용 확인만 해주시고 다음 미션부터 적용해주세요!

수고하셨습니다 👍

public int multipleNumbers(){
return num1 * num2;
}
}

Choose a reason for hiding this comment

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

scanner.close()는 resource를 풀어주는 거라 다른 결이긴 합니다!
해당 링크 참고해보시면 좋을 것 같아요!

}
return sum(listToInt(splitInput(input)));
}
}

Choose a reason for hiding this comment

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

여기도 설정해서 수정해주세요~

Copy link
Author

Choose a reason for hiding this comment

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

보내주신 내용 확인해서 파일마다 끝에 한 줄 비워두는 방식으로 수정했습니다!


public class InputView {

private Scanner scanner;

Choose a reason for hiding this comment

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

private,
private final,
private static final,
private static

각각 차이는 무엇일까요?

Copy link
Author

Choose a reason for hiding this comment

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

private는 해당 클래스 내부에서만 접근 가능하게 만들어주는 접근 제어자 입니다.
private final은 한 번 값이 설정되면 그 값이 고정되며 인스턴스 별로 각기 다른 값을 가질 수 있습니다.
그리고 private에 의해 해당 클래스 내부에서만 접근 가능합니다.
private static final은 모든 인스턴스가 사용하는 고정된 값을 할당할 때 사용합니다. 주로 상수 설정에 사용합니다.
private static은 객체 생성 없이 사용 가능한 필드나 메서드들을 생성하고 싶을 때 사용합니다.
클래스의 모든 인스턴스가 접근 가능하지만 private에 의해 해당 클래스 내부에서만 접근 가능합니다.

위와 같이 이해하고 있는데 잘못된 부분이 있을까요?

Comment on lines 23 to 26
while (scanner.hasNextLine()) {
return scanner.nextLine();
}
return null;

Choose a reason for hiding this comment

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

return scanner.next(); 이렇게 해도 될 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

EOF를 잘못 이해하고 나름대로 resource를 풀어주려고 작성했습니다.
보내주신 내용 확인하고 수정 완료했습니다!

Comment on lines 13 to 14
private static final int EXPECT_VALUE = 3;
private Calculator calculator;

Choose a reason for hiding this comment

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

Suggested change
private static final int EXPECT_VALUE = 3;
private Calculator calculator;
private static final int EXPECT_VALUE = 3;
private Calculator calculator;

Copy link
Author

Choose a reason for hiding this comment

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

수정 완료했습니다!

Comment on lines +24 to +25
assertThat(EXPECT_VALUE_ZERO).isEqualTo(calculator.add(null));
assertThat(EXPECT_VALUE_ZERO).isEqualTo(calculator.add(""));

Choose a reason for hiding this comment

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

지금은 위에 테스트 메서드가 실패하면 아래는 작동하지 않아서 여러 값을 테스트한다면 softly로 적용하는 것이 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

확인하고 수정하였습니다!

@mlngwan
Copy link
Author

mlngwan commented May 22, 2024

2차 Code Refactoring 진행하였습니다.

  • EOF 관련

    • 기존의 코드에서는 더 이상 받을 resource가 없다는 의미의 코드로 작성했습니다.
    • 각 파일 끝에 한 줄을 비워두는 방식으로 해결했습니다.
  • Test 관련

    • 기존의 코드에서는 만약 테스트가 실패한다면 실패한 지점에서 테스트가 멈춰버리게 되는 문제점이 있었습니다.
    • 따라서 실패 여부와 상관없이 모든 테스트를 실행하도록 문제점을 해결하였습니다.

Refactoring 과정에서 배운 내용

  • EOF (End OF File)

    • 과거 IEEE에서 책정한 POSIX에 의해 파일 끝에는 newline 문자가 존재해야 합니다.
    • 한 행씩 읽는 과정에서 newline 문자가 없으면 끝나지 않은 행으로 간주하기 때문입니다.
    • 요즘에는 위 과정이 없어도 잘 작동하지만 혹시 모를 오류나 관례(?)로 사용합니다.
    • 참고 : https://hjuu.tistory.com/8, https://velog.io/@doondoony/posix-eol
  • Test - Softly

    • assertThat(...)은 테스트에 실패하게 되면 실패를 알리고 테스트를 중지합니다.
    • 테스트에 실패하는 경우에도 밑의 테스트까지 시도하기 위해 SoftAssertions.assertSoftly(...)을 사용해야 합니다.
    • 테스트마다 SoftAssertions 객체를 생성해줘야 하는데 이를 @InjectSoftAssertions을 통해 간결하게 할 수 있습니다.
    • 참고 : https://jiwondev.tistory.com/186

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

Successfully merging this pull request may close these issues.

2 participants