Conversation
| if (nums.length <= k) { | ||
| return nums; | ||
| } |
There was a problem hiding this comment.
nums.length <= kの場合でも、ユニーク化した配列を返してあげる方が仕様として親切だと思いました。
条件分岐をもう少し後にし、たとえば：
if (numToCount.size() <= k) {
int[] all = new int[numToCount.size()];
int i = 0;
for (int key : numToCount.keySet()) all[i++] = key;
return all;
}There was a problem hiding this comment.
ユニーク化した配列を返してあげる方が仕様として親切
おっしゃる通りです。現状は期待されている出力になっておらず不適切ですね。
ご指摘ありがとうございます!
| PriorityQueue<NumAndFrequency> topKNumToCountEntry | ||
| = new PriorityQueue<>(Map.Entry.comparingByValue()); |
There was a problem hiding this comment.
PriorityQueue<NumAndFrequency>は誤植でしょうか。
There was a problem hiding this comment.
おっしゃる通りです。
PriorityQueue<Map.Entry<Integer, Integer>>が正しいです。。。
お恥ずかしい限りです。
|
|
||
| ## 解答 | ||
| - Step1のPriorityQueueを使った実装とほぼ一緒 | ||
| - `numToCountEntry`としているが、命名がいまいちな感じ |
| } | ||
|
|
||
| int[] result = frequentNums.stream() | ||
| .mapToInt(Map.Entry::getKey).toArray(); |
There was a problem hiding this comment.
メソッドチェーンを書く場合は、呼び出しごとに改行を入れたほうが、読み手にとって読みやすくなるかもしれません。
| // 要素が一つしかない場合 | ||
| if (left == right) return; | ||
|
|
||
| // (n - k)番目に頻度が高い値(kSmallestと一致する値)を選べればベストだが |
There was a problem hiding this comment.
ありがとうございます。少し調べてみました。
- 先頭、末尾、中央などいずれかの位置にある要素の値を採用する
- 乱数で適当に選択した要素の値を採用する
- いくつか取り出した要素の中央値を採用する(median of threeなど)
- 要素をいくつかのグループに分割し、それらの中央値の中央値を採用する(median of medians)
多くの場合は、ランダム/median of threeを使用し、超大規模データの場合はmedian of mediansを使用することもあると理解しました。
| // (n - k)番目に頻度が高い値(kSmallestと一致する値)を選べればベストだが | ||
| // 分からないので、ピボットをランダムに選ぶ | ||
| Random random = new Random(); | ||
| int pivotIndex = left + random.nextInt(right - left + 1); |
There was a problem hiding this comment.
細かいですがleft/rightにindexを付けてないのでこっちもpivotで良いかも知れません
There was a problem hiding this comment.
たしかに。統一されていないと実はindexではないのかと勘繰ってしまいますね。
統一するように心がけます。
| .toArray(NumAndFrequency[]::new); | ||
|
|
||
| // 頻度の高い要素を配列の(n - k)番目からn番目に集める | ||
| quickselect(numAndFrequencies, 0, n - 1, n - k); |
There was a problem hiding this comment.
再帰で書くには今の引数の方が都合がよく、n-kを渡してしまいました。
でも、呼び出す人の気持ちを考えたらkを渡す方が直感的で親切ですね。
オーバロード的な感じで(array、left、right、k)を渡す関数を用意しておけばよかったです。
|
|
||
| private void quickselect(NumAndFrequency[] numAndFrequencies, int left, int right, int kSmallest) { | ||
| // 要素が一つしかない場合 | ||
| if (left == right) return; |
4 participants
今回解いた問題：
347. Top K Frequent Elements
次に解く問題：
373. Find K Pairs with Smallest Sums