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

Open
wants to merge 19 commits into
base: hi-june
Choose a base branch
from

Conversation

hi-june
Copy link
Member

@hi-june hi-june commented Jun 12, 2023

📌 과제 설명

사용자가 입력한 중위 표현식을 기반으로 계산 결과를 보여주는 계산기 과제를 수행했습니다.

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

사용자에게 계산기 선택 메뉴를 보여줍니다.
사용자가 선택한 옵션에 따라 다른 기능을 합니다.(1. 계산 이력 조회, 2. 계산)
사용자에게 중위 표현식을 입력 콘솔을 통해 입력받을 수 있습니다.
사용자가 입력한 값이 계산기에서 유효한 값인지 검증하여, 유효하지 않을 시 예외 메시지를 출력합니다.
사용자가 입력한 중위 표현식이 유효한 중위 표현식인지 검증하여, 유효하지 않을 시 예외 메시지를 출력합니다.
사용자가 입력한 중위 표현식이 유효할 경우, 해당 표현식을 기반으로 계산하여 그 결과값을 출력합니다.
사용자가 실행동안 입력했던 계산 이력에 대해서 순서대로 보여줍니다.

최대한 함수는 한 가지 기능만 하도록 구현했습니다.
메모리 같은 경우 외부에서 직접 접근이 불가능하도록 지정된 메소드로만 저장 및 조회가 가능하도록 만들었습니다.
계산 이력을 확인하는 것은 이력이 나오는 순서같은 경우 요구사항이 변경될 수도 있으니, hashMap으로 구현하고, 요구 상황에 맞춰 keySet을 적절히 활용하는 방식으로 구현했습니다.
사용자의 입력을 검증하는 것을 코드로 하나하나 구현하기에는 너무 복잡해지기에 정규표현식을 활용했습니다.

✅ PR 포인트 & 궁금한 점

  1. static을 쓰는 이유가 궁금합니다. 관리되는 메모리 공간이 다르다는 점.. 정도만 이해하고 있습니다.
  2. 이번에는 switch문을 많이 썼는데, if-else if로도 만들 수 있을 것 같던데, 둘의 차이가 있을까요? 예전에 우테코 프리코스 과제에서 switch문을 사용하는 것을 제한하는 조건도 있었던 적이 있어서 혹시 유불리가 있는건지 궁금합니다.
  3. 문자열 포맷팅 같은 경우는 하드코딩으로 놔뒀는데, 어느 정도까지 상수로 따로 관리해야하는 것이 좋을까요?
  4. 주석은 너무 많은 것도 좋지 않다고 하던데, 어느 수준까지 다는게 좋을까요?
  5. Test 코드를 처음 짜 봤는데, 이런 식으로 짜는게 맞는건지.. 잘 모르겠습니다.
  6. OOP 설계적인 부분 피드백 받을 수 있다면 좋겠습니다.

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.

계산기 과제 고생하셨습니다! 간단하게 답변 남겨드립니다.

  • 마지막 줄을 다 비운 이유가 있을까요? 일반적으론 마지막에 개행문자를 넣는편이라 궁금증으로 여쭤봅니다!
  • static은 메모리를 좀 더 효율적으로 쓰기위해 씁니다! 컴파일타임에 쓰레드끼리 공유하는 영역을 만들어 변경이 없는 변수나, 메서드에서 자주 사용합니다. 따라서 변수의경우 final과 자주 같이 쓰는 이유는 final로 변경도 못하게 한다면 상수로 취급하여 효율적으로 사용할 수 있다고 알고있습니다
  • 문자열 포매팅이 어디말하는걸까요?
  • 주석은 팀마다 다르긴 합니다. 목적에 따라도 다르다고 생각합니다. 하지만 자주 변경이 있고 같이 협업하여 관리하는 웹 애플리케이션이라면 최대한 관리포인트는 줄이는게 좋기 때문에 주석이 없는게 가장 좋다고 생각합니다! 만약 설명없이 이해할 수 없는 코드라면 리팩터링이 필요하다고 생각합니다.
    • 웹 애플리케이션에서 주석을 사용한다면 안좋아보이지만, 꼭 써야만 하는 코드가 있는 경우에 쓸 듯 하네요!
    • 프레임워크나 라이브러리는 자바독을 쓰는게 일반적입니다.
    • 나중에 좀 더 찾아보시면 좋을것같아여. 제 지식은 여기가 한계입니다..
  • TestCode는 괜찮아 보입니다! 나중에 좀 더 공부하면서 익히면 좋을것같습니다. 지금은 간단한 어플리케이션이라 이정도면 충분해보이네여. 추가적으로 테스트 코드마다 케이스를 모두 다르게 해서 만드는 경우도 있고 그렇습니다. 현재는 모든 케이스를 몰아서 쓰셔서 말씀드려봅니다
  • 만약 처음부터 OOP를 고려하고 짠다면, Interface를 많이 활용할 것 같습니다. main에서 주입해주고, 내부 코드는 확장하기 전까지 비즈니스로직을 수정하지 않도록 유지할 것 같네요! (IO라던가 Repository 등)

Comment on lines +40 to +42
case ADD_OPERATOR:
case SUBTRACTION_OPERATOR:
case MULTIPLY_OPERATOR:
Copy link

Choose a reason for hiding this comment

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

안쓰는데 명시한 이유가 있을까요?? 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

모든 사칙 연산자가 들어올 경우에 동일한 동작을 정의하기 위해 사용했습니다.
사칙 연산자 중 하나의 연산자가 들어올 경우 44~47번 까지 동일한 코드가 수행되고, break문을 만나는 방식으로, 사칙연산자와 괄호, 그 외의 경우로 구분하였습니다.

* @throws IllegalArgumentException
*/
public String convert(String inputExpression) throws IllegalArgumentException {
StringBuilder sb = new StringBuilder();
Copy link

Choose a reason for hiding this comment

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

네이밍 구체적으로 가능할까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

함수 네이밍 말씀이시죠? 수정하겠습니다!

* @param expression 표현식
* @throws IllegalArgumentException
*/
private void validateConsecutiveOperators(String expression) throws IllegalArgumentException {
Copy link

Choose a reason for hiding this comment

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

expression을 바로 matches하는 비용이 크기 때문에 compile해놓고 사용하는게 좋아보입니다! Pattern.compile 찾아 보심 좋아보여요

Comment on lines +51 to +53
} catch (Exception e) {
ioConsole.print(e.getMessage() + "\n");
}
Copy link

Choose a reason for hiding this comment

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

나중에 �custom exception도 고려해서 짜보면 좋을것같습니다!

Comment on lines +40 to +41
@Test
void convertInValidExpressionTest() {
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

고생 하셨습니다. 클래스를 적절히 잘 나눠서 사용하신 것 같아요.
훈기님이 리뷰에 덧붙일 내용만 추가 할게요.

  • 불 필요한 개행은 삼가해 주세요.
  • if문의 조건에 ! 을 최대한 안쓰고 메소드 명으로만 읽히게 만드는 습관을 들이시면 더 좋을 것 같아요.
  • 테스트에 assertEquals(true, result) 는 assertTrue(result) 로 사용할 수 있습니다.

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