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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

#1

wants to merge 1 commit into from

Conversation

chenghaLim
Copy link
Owner

}

public static void printRevenueRate(double EarningRate) {
System.out.println("총 수익률은 " + String.format("%.1f", EarningRate) + "%입니다.");

Choose a reason for hiding this comment

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

String.format("총 수익률은 %.1f%%입니다.", EarningRate);

하나로 format를 해줬으면 가독성이 더 좋았을 것 같아요

@@ -0,0 +1,63 @@
package Controller;

Choose a reason for hiding this comment

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

패키지명은 보통 다 소문자로 작성해요
자바 컨벤션을 읽어보는 걸 추천드려요~


// 상금을 카운트해서 넣을 Map
private Map<Reward, Integer> resultList() {
Map<Reward, Integer> result = new LinkedHashMap<>();

Choose a reason for hiding this comment

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

LinkedHashMap을 사용한 이유가 순서 유지 때문이라면 Map활용을 잘 하셨네요! 👍

printLottoList(lottoList, money.quantity());

Print.userLotto();
String winningNums = Input.inputWinLottoNum();

Choose a reason for hiding this comment

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

요구 사항 중 입력 예외 발생 시 에러 메시지를 출력 후 그 부분부터 입력을 다시 받는다.
이 부분의 구현이 없어서 조금 아쉬운 것 같아요

Comment on lines +60 to +62
for (int i = Reward.values().length - 1; i >= 0; i--) {
Reward.values()[i].printMessage(result.get(Reward.values()[i]));
}

Choose a reason for hiding this comment

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

foreach를 사용했으면 좀 더 가독성 좋은 코드로 구현하지 않았을까 싶어요
그리고 values()에 배열로 접근하는 부분이 좀 아쉬운 것 같아요

@DisplayName("당첨금 횟수 세기")
@Test
void lottoResult() {
List<Lotto> lottoList = List.of(

Choose a reason for hiding this comment

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

Collection을 점점 더 잘 활용하시는 것 같아요 💯

import java.util.List;
import java.util.Map;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;

Choose a reason for hiding this comment

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

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
위의 클래스보단 아래의 클래스를 좀 더 사용하는 것 같아요
import static org.assertj.core.api.Assertions.assertThat;

Map<Reward, Integer> result = new HashMap<>();
result.put(Reward.FIRST, 1);

assertEquals(2.0E8, controller.rateOfReturn(result, 1));

Choose a reason for hiding this comment

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

double로 소수점 표현 시 부동소수점 문제로 인해 오차 발생이 일어나요(수가 크면)
지수로 나타내기보단 rateOfReturn()에서 반올림을 했으면 더 좋았을 것 같아요

부동소수점에 대해서 찾아보면 좋을 것 같아요
금액과 관련된 서비스 시 관련 문제를 볼 수도 있을 것 같아요

Comment on lines +10 to +15
@DisplayName("입력한 금액이 숫자가 아닐때")
@Test
void inputMoneyNotNum() {
Assertions.assertThatThrownBy(() -> new Money("가나다"))
.isInstanceOf(IllegalArgumentException.class);
}

Choose a reason for hiding this comment

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

실패에 대한 케이스도 정말 중요하지만,
성공에 대한 케이스도 작성해두면 좋을 것 같아요

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