Conversation
hayashi-ay
left a comment
hayashi-ay
left a comment
There was a problem hiding this comment.
良いと思います。quickselectのpivotの選択は、真ん中の値を取る以外にも乱択や3 median（先頭、真ん中、末尾のメジアン）などもあります。
|
Issueのタイトルでどの問題かが分かるかがタイトルも含めてあると良いかなと思いました。 |
| 他の人のレビューをみてみると関連知識としてQuickSelectの解法もあったので、勉強のためにそちらでも解いてみた。 | ||
| https://github.com/Ryotaro25/leetcode_first60/pull/10#discussion_r1627890286 | ||
| 以前他の問題でもQuickSelectをやっていたがまだ身に付いてはいなかったので実装に15分ほど時間がかかってしまった。 | ||
| K番目までみたいにわかりやすい条件のときは、すぐに知識がリンクするようにしたい。 |
There was a problem hiding this comment.
Java を使う場合は、TreeMap も見ておいてください。私は、PriorityQueue よりもこちらのほうが大事な感覚があります。Key が順番に並んでいます。
https://docs.oracle.com/javase/7/docs/api/java/util/TreeMap.html
| if (pivotIndex == target) { | ||
| return; | ||
| } else if (pivotIndex < target) { | ||
| left = pivotIndex + 1; | ||
| } else { | ||
| right = pivotIndex - 1; | ||
| } |
There was a problem hiding this comment.
好みの問題ですが、pivotIndex == target のケースだけ終了条件になっているので、そこだけ別の if 文にしてもいいかなとおもいました。
|
|
||
| private void quickSelect(int left, int right, List<Map.Entry<Integer, Integer>> frequency, int target) { | ||
| while(left < right) { | ||
| int pivotIndex = left + (right - left) / 2; |
There was a problem hiding this comment.
pivotIndex を算出する方法はこれ以外にもいくつかあるので、もしご存知なければ他の方の解答等を見て把握しておくと良いと思いました。それらを踏まえた上で、なぜこの方法を選ぶのかというのが意識できていると素晴らしいと思います！
| storeIndex++; | ||
| } | ||
| } | ||
| Collections.swap(frequency, storeIndex, right); |
There was a problem hiding this comment.
Collections.swap() 知りませんでした。勉強になります！✍️
| minCountAndNums.add(entry); | ||
| if (k < minCountAndNums.size()) { | ||
| minCountAndNums.poll(); | ||
| } |
There was a problem hiding this comment.
k 個既に要素がある場合、一番頻度が低い要素よりも entry の頻度がさらに低い場合スキップすれば、ケースによっては処理時間を結構減らせそうです
|
バケットソートを用いて計算量 O(n) で実装する方法もあります。他の方のプルリクにあったと思うので、目を通しておいても損はないと思います。 |
|
↑ Pythonですが自分Bucket Sortでも解いてます。https://github.com/hayashi-ay/leetcode/pull/60/files |
| numToCount.put(num, numToCount.getOrDefault(num, 0) + 1); | ||
| } | ||
|
|
||
| List<Map.Entry<Integer, Integer>> frequency = new ArrayList<>(numToCount.entrySet()); |
There was a problem hiding this comment.
Map.Entry を pair や tuple の代わりに使っている点に違和感を感じました。クラスを定義し、クラスのインスタンスのリストとしたほうが良いと思います。
There was a problem hiding this comment.
Java 16 で正式リリースされた Record の使用もおすすめします。
https://docs.oracle.com/en/java/javase/17/language/records.html
| Collections.sort(elements); | ||
| int[] topKFrequentElements = new int[k]; | ||
| for(int i = 0; i < k; i++) { | ||
| topKFrequentElements[i] = (int)elements.get(elements.size() - i - 1).value; | ||
| } |
There was a problem hiding this comment.
降順ソートしてelements[i].valueで抜き出した方が自然だと思います
| for (int i = 0; i < k; i++) { | ||
| topKFrequentElements[i] = (int) numToCountQueue.poll().getKey(); | ||
| } |
There was a problem hiding this comment.
forでも問題ありませんが、iterator を使っても良さそうです
- for文より条件と処理を簡潔に書ける
- 仮にnumsがk以下でも安全に動作する
などのメリットがあると思いました
6 participants
問題URL: https://leetcode.com/problems/top-k-frequent-elements/description/