347. Top K Frequent Elements #10
Conversation
| heapq.heappush(sorted_nums, (freq, num)) | ||
|
|
||
| top_k_freq = [] | ||
| while(len(sorted_nums) > k): |
There was a problem hiding this comment.
あんまり括弧は使うな、というコーディングスタイルもあるみたいです。
https://google.github.io/styleguide/pyguide.html#33-parentheses
少なくとも、whileと(の間には1つスペースがあるといい様に思いました。
| for num in nums: | ||
| num_to_count[num] += 1 | ||
|
|
||
| sorted_nums = [] |
There was a problem hiding this comment.
確かに、何でソートしているのかがわからないですし、なんか変ですね。
There was a problem hiding this comment.
同じ感想を持ちました。（heapq で使われている）min-heap では root が最小値であることが保証されているだけなので sorted とは言えなさそうです
| for num, freq in num_to_count.items(): | ||
| heapq.heappush(sorted_nums, (freq, num)) | ||
|
|
||
| top_k_freq = [] |
| heapq.heappush(elements_order_by_freq, [value, key]) | ||
|
|
||
| while len(elements_order_by_freq) > k: | ||
| heapq.heappop(elements_order_by_freq) |
There was a problem hiding this comment.
if len() > k の時点で取り出すとメモリが少なく済みそうです。
for key, value in elements.items():
heapq.heappush(elements_order_by_freq, [value, key])
if len(elements_order_by_freq) > k:
heapq.heappop(elements_order_by_freq)| num_to_count[num] += 1 | ||
|
|
||
| bucket = [[] for _ in range(len(nums) + 1)] | ||
| for num, freq in elements.items(): |
There was a problem hiding this comment.
こちら elements -> num_to_count ですね。
| elements[num] = 0 | ||
| elements[num] += 1 | ||
| ``` | ||
| - クイックセレクトを使った解法もありましたがこれくらいは"常識"なんですかね? |
There was a problem hiding this comment.
Quick Sort は、授業でやるでしょう。Quick select は知っていると思います。
ただ、この問題が出題されたときに選択するかというと少し過剰に大変に思いますね。(そういう考え方も思いついたと口頭で言うくらい。)
| elements[num] += 1 | ||
| ``` | ||
| - クイックセレクトを使った解法もありましたがこれくらいは"常識"なんですかね? | ||
| - 他の方の解答で、returnのところに[num for num, freq in sorted\_num\_to\_freq\_list]みたいにして書いているのは個人的にはあまり好きじゃない(変数に格納してreturn 変数 みたいにしたい)が、これは個人の好みとして片付けて良いのか、"一般的な"ソフトウェアエンジニアが書く方法があるのか気になった。 |
There was a problem hiding this comment.
ある程度好みの範囲でしょうが、横に長くて特に順序があっちこっちいくやつは、目を移動させないといけないので読みにくいです。
https://discord.com/channels/1084280443945353267/1303257587742933024/1311262294457712652
There was a problem hiding this comment.
計算や処理をしているのを変数に格納せずにreturnしているのが個人的に嫌、という理由のほかに、そのような場合は目を移動させて読むことになることが往々にしてあるからというのも感じていました。
ちなみにこちらの方を見ていて質問したいと思ったのですが、PEP8だとlambdaは非推奨とされていると思います。実際にlambdaは使われているんですかね。そもそもlambdaを使ってコードを書くことは少ない(というよりかはpythonにはこんな機能もあります!というような紹介で使っているだけの感じがある)んですが、私が使っていないだけですかね。
There was a problem hiding this comment.
Python の考え方に
https://peps.python.org/pep-0020/
There should be one-- and preferably only one --obvious way to do it.
というのがあります。
これは、Perl の
There is more than one way to do it.
とは逆です。
lambda は、様々な言語にありますが、他言語と比較したときに「式であるなど、故意にかなり使いにくくしてある」が、あるとありがたいことが稀にあるので用意されている、くらいの温度感で私は見ています。
| - https://github.com/goto-untrapped/Arai60/pull/55 | ||
|
|
||
| 他の方のPRを見ていたときのメモ | ||
| - dictを使わなくても、Counterでできる https://docs.python.org/3/library/collections.html#collections.Counter |
There was a problem hiding this comment.
https://docs.python.org/3/library/collections.html#collections.Counter.most_common
most_common の実装を見てもいいかもしれません。
| for num in nums: | ||
| num_to_count[num] += 1 | ||
|
|
||
| sorted_nums = [] |
There was a problem hiding this comment.
同じ感想を持ちました。（heapq で使われている）min-heap では root が最小値であることが保証されているだけなので sorted とは言えなさそうです
| for num in sorted_nums: | ||
| top_k_freq.append(num[1]) |
There was a problem hiding this comment.
num で受け取って2つ目の数を append するという処理をしていますが、for 時点で展開して受け取ることができたと思います。
| for num in sorted_nums: | |
| top_k_freq.append(num[1]) | |
| for _, num in sorted_nums: | |
| top_k_freq.append(num) |
| if len(sorted_nums) >= k: | ||
| return sorted_nums[:k] |
There was a problem hiding this comment.
num の種類が k 未満の場合には、return に到達せずに for ループを終えるので None を返してしまいそうです。
k 未満の場合の対応をどうするかにもよりますが、ある分だけ返すようにするのであればこんな感じがよいかもです。またその場合にはスライスせず sorted_nums で十分そうです。
| if len(sorted_nums) >= k: | |
| return sorted_nums[:k] | |
| if len(sorted_nums) >= k: | |
| break | |
| return sorted_nums |
No description provided.