Skip to content

Conversation

jaejeong1
Copy link
Contributor

@jaejeong1 jaejeong1 commented Aug 15, 2024

@jaejeong1 jaejeong1 added the java label Aug 15, 2024
@jaejeong1 jaejeong1 self-assigned this Aug 15, 2024
@dev-jonghoonpark
Copy link
Contributor

코드 마지막 라인에 line break 추가해주시면 좋을 것 같습니다!
(일반적으로 ide, editor 에 옵션으로도 제공됩니다.)

.toArray();

// 배열에서 상위 k개만 반환
return Arrays.copyOf(sortedNums, k);
Copy link
Contributor

Choose a reason for hiding this comment

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

재밌는 코드 잘 보았습니다 : )

풀이에 영향이 있는건 아니지만 궁금한 부분이 있어서 질문드립니다!

// 빈도순 정렬 후 상위 k개만 배열로 반환
return freq.keySet().stream()
        .sorted((a, b) -> freq.get(b) - freq.get(a))
        .mapToInt(i -> i)
        .limit(k)
        .toArray();

이런식으로 limit 을 걸어줘도 됐을 것 같은데
혹시 Arrays.copyOf를 추가적으로 사용하신 이유가 있으신지 궁금합니다 : )

Copy link
Contributor

Choose a reason for hiding this comment

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

좋은 피드백 감사합니다. 두 방법 모두 결과는 같으나
copyOf 사용 시 새 배열 생성으로 인해 불필요한 메모리를 낭비 및 성능적 단점이 있어 보이네요 :)
제안주신 방법이 더 나은 방안으로 보여 피드백 적용해보겠습니다!!

@jaejeong1 jaejeong1 marked this pull request as ready for review August 16, 2024 14:52
Comment on lines +18 to +23
return freq.keySet().stream()
.sorted((a, b) -> freq.get(b) - freq.get(a))
.mapToInt(i -> i)
.limit(k) // 배열에서 상위 k개만 반환
.toArray();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Stream 사용하니까 엄청 직관적으로 정렬 시킬 수 있네요.. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 맞습니다!! 스트림의 장점인것 같아요 ㅎㅎ

Copy link
Member

@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.

수고하셨습니다. 모임 전 PR 병합 부탁드리겠습니다.

@jaejeong1 jaejeong1 merged commit f08dc73 into DaleStudy:main Aug 18, 2024
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.

5 participants