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

[정현준] 3주차 답안 추가 #390

Merged
merged 2 commits into from
Aug 30, 2024
Merged

[정현준] 3주차 답안 추가 #390

merged 2 commits into from
Aug 30, 2024

Conversation

jdalma
Copy link
Member

@jdalma jdalma commented Aug 27, 2024

답안 제출 문제

체크 리스트

  • PR을 프로젝트에 추가하고 Week를 현재 주차로 설정해주세요.
  • 바로 앞에 PR을 열어주신 분을 코드 검토자로 지정해주세요.
  • 문제를 모두 푸시면 프로젝트에서 Status를 In Review로 설정해주세요.
  • 코드 검토자 1분 이상으로부터 승인을 받으셨다면 PR을 병합해주세요.

@jdalma jdalma requested a review from a team as a code owner August 27, 2024 08:59
@jdalma jdalma requested a review from kim-young August 27, 2024 09:00
@jdalma jdalma added the kotlin label Aug 27, 2024
Comment on lines +27 to +30
climbStairs(1) shouldBe 1
climbStairs(2) shouldBe 2
climbStairs(3) shouldBe 3
climbStairs(4) shouldBe 5
Copy link
Contributor

Choose a reason for hiding this comment

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

역시 kotest 가 테스트 코드 가독성이 정말 뛰어난 것 같습니다!

Comment on lines 37 to 39
if (map.containsKey(diff)) {
return intArrayOf(map[diff]!!, index)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

kotlin 의 null safety 장점을 살리기 위해 아래처럼 safe call operator(?.) 를 사용해보면 어떨까요?

map[diff]?.let {
    return intArrayOf(it, index)
}

NPE 를 선호하신다면 not-null assertion operator(!!) 를 사용하셔도 되나 코틀린의 장점을 생각할 때 보통 지양하는 편이긴 합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

리뷰 감사합니다 ㅎㅎ !!연산은 실무에서는 절대 사용하지 않는 편인데 이 문제에서는 절대 비어있지 않을거라고 생각하고 사용했었네요 ㅠ 말씀하신대로 수정해봤습니다. 버릇을 잘 들여놓아야겠습니다 ㅎㅎ

Copy link
Contributor

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

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

수고하셨습니다. Kotlin 파이팅!

@jdalma jdalma merged commit a4ee01e into DaleStudy:main Aug 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

3 participants