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

Step2 - 문자열 계산기 #1151

Open
wants to merge 6 commits into
base: kimbacksul
Choose a base branch
from

Conversation

kimbacksul
Copy link

@kimbacksul kimbacksul commented Jun 11, 2023

Step2 - 문자열 계산기 리뷰요청 드립니다.

Step1 실습 코드 페키지 이동 시키면서 추가 커밋이 발생했는데 아래 2개 파일은 무시해주세요.
(처음 리뷰시 전달 해주신 필요없는 주석 제거 이외에 수정 사항은 없습니다. )

  • src/main/kotlin/Person.kt
  • src/test/kotlin/PersonTest.kt

Step 2 관련 리뷰요청을 위해 추가된 파일은 아래 2가지 입니다.

  • src/main/kotlin/Calculation.kt
  • src/test/kotlin/CalcutationTest.kt

Copy link

@ujusy ujusy left a comment

Choose a reason for hiding this comment

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

안녕하세요 ~ 2단계 구현하시느라 고생하셨습니다 😊
몇 가지 고민해볼 코멘트를 남겼습니다!
확인 후 다시 리뷰 요청 주세요 🙌

그리고 미션을 진행할 때 구현에 앞서 미션 요구 사항을 README.md에 정리해보면 좋을 것 같아요 :)

private fun divide(first: Int, second: Int): Int {
return first / second
}
}
Copy link

Choose a reason for hiding this comment

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

intellji에서 new line 설정이 가능하니 설정해두시면 좋습니다 :)

return first * second
}

private fun divide(first: Int, second: Int): Int {
Copy link

Choose a reason for hiding this comment

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

5/2 일 경우에는 어떤 결과값이 나올까요? 이 경우도 테스트로 작성해보면 좋을 것 같아요:)

Comment on lines +4 to +6
if(input.isNullOrBlank()) {
throw IllegalArgumentException()
}
Copy link

Choose a reason for hiding this comment

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

kotlinruquire를 사용해 볼 수 있어요 :)


fun calcutateByString(input: String?): Int {
if(input.isNullOrBlank()) {
throw IllegalArgumentException()
Copy link

Choose a reason for hiding this comment

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

명시적인 오류 메시지를 던져보는건 어떨까요?

if(input.isNullOrBlank()) {
throw IllegalArgumentException()
}
val splitedInput = input.split(" ")
Copy link

Choose a reason for hiding this comment

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

" " 는 상수화 해 볼 수 있을 것 같아요 :)

throw IllegalArgumentException()
}
val splitedInput = input.split(" ")
var result: Int = splitedInput.first().toInt()
Copy link

Choose a reason for hiding this comment

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

숫자가 아닌 문자가 들어오면 어떻게 될까요?


@Test
fun `예제 확인`() {
assertThat(Calculation().calcutateByString("2 + 3 * 4 / 2"))
Copy link

Choose a reason for hiding this comment

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

여유가 있다면 @ParameterizedTest 를 이용해서 다양한 케이스에 대해 테스트를 해보면 좋을 것 같아요 :)

@@ -0,0 +1,40 @@
class Calculation {

fun calcutateByString(input: String?): Int {
Copy link

Choose a reason for hiding this comment

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

이를 직접 콘솔에서 실행해 볼 수 있는 main함수를 만들어보면 어떨까요? :)

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