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

[bus710] Week 09 #984

Merged
merged 1 commit into from
Feb 8, 2025
Merged

[bus710] Week 09 #984

merged 1 commit into from
Feb 8, 2025

Conversation

bus710
Copy link
Contributor

@bus710 bus710 commented Feb 2, 2025

답안 제출 문제

체크 리스트

  • 우측 메뉴에서 PR을 Projects에 추가해주세요.
  • Projects의 오른쪽 버튼(▼)을 눌러 확장한 뒤, Week를 현재 주차로 설정해주세요.
  • 바로 앞에 PR을 열어주신 분을 코드 검토자로 지정해주세요.
  • 문제를 모두 푸시면 프로젝트에서 StatusIn Review로 설정해주세요.
  • 코드 검토자 1분 이상으로부터 승인을 받으셨다면 PR을 병합해주세요.

@bus710 bus710 requested a review from a team as a code owner February 2, 2025 04:43
@github-actions github-actions bot added the go label Feb 2, 2025
@bus710 bus710 requested a review from gwbaik9717 February 2, 2025 04:43
@bus710
Copy link
Contributor Author

bus710 commented Feb 2, 2025

문제 1을 두번 풀어 보았습니다.

첫번째 풀이가 성능이 너무 안 좋은 것 같아서, 두번째는 단순하게 루프에 돌려보았습니다.
두번째 풀이의 성능이 나아지긴 했지만 아쉽게도 큰 차이는 없네요. 어떤 부분이 문제일지 ㅎㅎ

암튼 간에, 시간/공간 복잡도는 둘 다 O(n)이라고 생각 합니다. 주어진 리스트의 노드 수 만큼, 맵의 아이템 수가 증가하기 때문에, 그리고 재귀던 루프던 한번씩은 다 방문하게 짜놓았기 때문에요.

(궁금해서 솔루션을 찾아 보았습니다. 생각보다 간단한 방법으로 두배 이상 빠르게 풀 수 있는 방법이 있었네요. 리뷰어분들께 스포가 될 수 있으니 적지 않도록 하겠습니다.)

Copy link
Contributor

@gwbaik9717 gwbaik9717 left a comment

Choose a reason for hiding this comment

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

안녕하세요, @bus710 님! 이번 리뷰는 조금 늦었습니다. 코멘트 몇 개 남겨 두었으니 시간되실 때 확인해주시면 좋을 것 같습니다. 이번주도 수고하셨습니다!

return false
}
m := map[*ListNode]int{}
for head.Next != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

for 문의 조건에 head.Next != nil 가 아닌 head != nil 로 하면, 31 번째 줄의 if 문이 필요없게 되어서 코드가 조금 더 깔끔해지지 않을까 생각이 들었습니다. 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

첫번째 조건은 사실 제출하면서 추가 된 에러를 핸들링하느라 붙였더니 그렇게 통합 될 수도 있을 것 같네요.

m := map[*ListNode]int{}
for head.Next != nil {
if _, ok := m[head]; !ok {
m[head] = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

현재 선언해두신 m 은 key 의 존재 여부만 확인하면 되기 때문에 value 의 역할은 사실상 큰 의미가 없어 보입니다. 저는 go 언어를 잘알지는 못하지만, int 대신 struct{} 를 사용하면 별도의 메모리 공간 할당이 필요없다고 하더라구요. 즉, m := map[*ListNode]struct{}{}m 의 타입을 변경할 수도 있을 것 같습니다. 물론, 그럼에도 메모리상 큰 차이는 없을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 저도 들어본 것 같은 얘기네요. 아마도 포인터로 다루기 때문에 구조체를 값으로 쓰면 공간이 덜 든다고 했던 것 같기도...

@bus710 bus710 merged commit e9cc8ac into DaleStudy:main Feb 8, 2025
3 checks passed
@bus710 bus710 deleted the week09 branch February 8, 2025 23:06
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.

2 participants