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

[정현준] 11주차 #549

Merged
merged 3 commits into from
Oct 27, 2024
Merged

[정현준] 11주차 #549

merged 3 commits into from
Oct 27, 2024

Conversation

jdalma
Copy link
Member

@jdalma jdalma commented Oct 22, 2024

답안 제출 문제

체크 리스트

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

@jdalma jdalma added the kotlin label Oct 22, 2024
@jdalma jdalma requested a review from obzva October 22, 2024 09:38
Comment on lines +13 to +16
fun maxDepth(root: TreeNode?): Int {
return if (root == null) 0
else max(maxDepth(root.left) + 1, maxDepth(root.right) + 1)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

간결하고 좋네요!

@jdalma jdalma marked this pull request as ready for review October 25, 2024 03:26
@jdalma jdalma requested a review from a team as a code owner October 25, 2024 03:26
*/
fun maxPathSum(root: TreeNode?): Int {
if (root == null) return 0
var max = root.`val` // 부모 노드와 2개의 자식 노드의 합을 전역 변수로 갱신한다.
Copy link
Contributor

Choose a reason for hiding this comment

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

사소한 내용이긴 한데, // 부모 노드와 2개의 자식 노드의 합을 전역 변수로 갱신한다. 라는 코멘트가 직관적으로 와닿진 않았습니다

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

@obzva obzva Oct 26, 2024

Choose a reason for hiding this comment

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

우선 대안 없이 비평만 남긴 점 사과 드립니다
빠르게 리뷰 남겨드리고 싶은 생각에 너무 대충 코멘트만 달아버린 것 같네요..

제가 고민했던 포인트는 이거였습니다

  • 부모 노드와 2개의 자식 노드의 합을 전역 변수로 갱신한다는 정확하고 자세한 표현임
  • 하지만 dfs를 수행하며 maxPathSum을 갱신한다같은 표현과 비교했을 때, 한 번에 와닿지 않는다고 느꼈음 (과장된 비슷한 예시: 피자를 만든다 vs 밀가루 반죽 위에 토마토 페이스트를 바르고 치즈를 뿌리고 화덕에 굽는다)
  • 대신 dfs를 수행하며 maxPathSum을 갱신한다는 앞선 표현보다는 덜 자세하고 덜 엄밀한 표현임

자세한 알고리즘 전개는 함수를 보면 자연스레 알 수 있으니까 코멘트로는 dfs를 수행하며 maxPathSum을 갱신한다와 같은 가이드를 해주는게 더 좋다고 생각했는데요, 이건 전적으로 제 개인적인 의견이라 사실 스킵하셔도 되는 부분입니다 :)

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

@obzva obzva left a comment

Choose a reason for hiding this comment

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

한 주 고생 많으셨습니다!

@jdalma jdalma merged commit 212bfe6 into DaleStudy:main Oct 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants