[sukyoungshin] WEEK 01 #1178
[sukyoungshin] WEEK 01 #1178
Conversation
top-k-frequent-elements/suKyoung.ts
Outdated
| if (heap.length > k) { | ||
| heap.shift(); | ||
| } |
There was a problem hiding this comment.
여기서는 heap의 크기가 그렇게 크지 않아서 큰 영향은 없을 것 같지만,,,
shift 메서드는 제거한 뒤에 남은 원소들을 앞쪽으로 당기기 때문에 시간복잡도가 O(n)으로 크기가 큰 배열에서는 성능이 좋지 않은 메서드라고 알고 있어요! (참고 블로그)
오름차순이 아닌 내림차순으로 정렬한 뒤에 heap.pop()으로 뒤쪽 원소를 제거한다면 좀 더 빠르게 적게 등장한 원소를 제거할 수 있을 것 같아서 코멘트 남겨봅니당 ㅎㅎ
two-sum/suKyoung.ts
Outdated
| const index = nums.indexOf(j); | ||
|
|
||
| if (nums.includes(j) && i !== index) { | ||
| return [i, index]; | ||
| } |
There was a problem hiding this comment.
indexOf 메서드는 만약에 j를 nums에서 찾지 못하면 -1을 반환하는 것으로 알고 있어요! (mdn 참고)
nums.includes(j)를 조건문에서 한번 더 쓰게 되면 nums에서 원소를 찾는 연산이 중복이 되는 것 같아서...
indexOf 의 결과가 -1인지 확인해주는 방법을 써도 좋을 것 같아요!!
top-k-frequent-elements/suKyoung.ts
Outdated
| function topKFrequent(nums: number[], k: number): number[] { | ||
| const frequencyMap = buildFrequencyMap2(nums); | ||
| const frequencyBuckets = buildFrequencyBuckets(nums.length, frequencyMap); | ||
|
|
||
| return collectTopKFrequent(frequencyBuckets, k); | ||
| } |
There was a problem hiding this comment.
알고리즘 풀이를 할 때에는 함수를 잘 분리하지 않게 되던데... 함수를 잘 분리해 주셔서 덕분에 긴 흐름을 잘 이해할 수 있었습니다 👍👍
two-sum/suKyoung.ts
Outdated
| } | ||
|
|
||
| return []; | ||
| }; |
There was a problem hiding this comment.
지금 linelint에서 에러가 나고 있는데 답안 제출 가이드를 보니 파일 끝에 개행문자를 추가해야 한다고 해요!
two-sum이랑 top-k-frequent-elements 쪽 파일 끝 개행문자 추가해주시면 에러도 해결될 듯 합니다 ㅎㅎ
|
꼼꼼하게 코드리뷰 해주셔서 감사합니다, @yoouyeon 님! |
답안 제출 문제
작성자 체크 리스트
In Review로 설정해주세요.검토자 체크 리스트
Important
본인 답안 제출 뿐만 아니라 다른 분 PR 하나 이상을 반드시 검토를 해주셔야 합니다!