Conversation
| class Solution { | ||
| public: | ||
| vector<int> topKFrequent(vector<int>& nums, int k) { | ||
| unordered_map<int, int> frequency; |
There was a problem hiding this comment.
個人的にmapのようなkey-value形式の変数名は{key}_to_{value}とすることが多いです。今回であればnum_to_countにするかなと思いました。変数名からどのような値が入っているのか分かりやすくなるという感覚です。
| vector<int> topKFrequent(vector<int>& nums, int k) { | ||
| unordered_map<int, int> frequency_to_idx; | ||
| priority_queue<pair<int, int>, vector<pair<int, int>>, greater<pair<int, int>>> kth_largest; | ||
| for (int num : nums) frequency_to_idx[num]++; |
There was a problem hiding this comment.
ぶら下がり文はバグの原因になる場合があります。 step2 以降のように { } で囲い、 1 文 1 行で書くほうがバグりにくいと思います。
There was a problem hiding this comment.
ぶら下がり文はついつい書いてしまうので注意します。
There was a problem hiding this comment.
事故の例があります。(ぶら下がりは絶対駄目とはいいませんが事故は時々あります。)
There was a problem hiding this comment.
たしかに例を見ると事故が起きることもありそうですね、気を付けます。
| class Solution { | ||
| public: | ||
| vector<int> topKFrequent(vector<int>& nums, int k) { | ||
| unordered_map<int, int> frequency_to_idx; |
There was a problem hiding this comment.
英単語から文字を削って識別子にすると、読み手の認知負荷が上がる場合があります。原則としてフルスペルで記述することをお勧めします。
一方で、情報科学やソフトウェアエンジニアリングの文脈で広く知られている略語（API, LAN, DNS, JSON など）や、所属チームの中で一般的に使われている略語であれば、使用しても問題ないと思います。
また、以下のような prefix は頻繁に目にするため、使用しても読み手が困ることは少ないと思います。
num_（number of）sum_（sum of）max_（maximum number of）min_（minimum number of）
参考までに、スタイルガイドへのリンクを共有します。
https://google.github.io/styleguide/cppguide.html#General_Naming_Rules
Do not abbreviate by deleting letters within a word.
なお、このスタイルガイドは唯一の正解というわけではなく、多くあるガイドラインの一つに過ぎません。所属するチームによって“良い書き方”の基準が異なることもありますので、ご自身の中で基準を持ちつつ、最終的にはチームの一般的な書き方に合わせることをお勧めします。
| frequency[num]++; | ||
| } | ||
| priority_queue<pair<int, int>, vector<pair<int, int>>, greater<pair<int, int>>> kth_largest; | ||
| for (auto& [num, count] : frequency) { |
There was a problem hiding this comment.
変数の型サイズが CPU のレジスタ幅以下の場合、値渡しにしたほうが効率的な場合があります。
for (auto [num, count] : frequency) {参照渡しにすると、値にアクセスする際に参照を経由する必要があり、アセンブリレベルでは間接アドレッシングが発生する可能性があります。これは直接アドレッシングに比べて余分な処理になることがあります。
もっとも、コンパイラの最適化によっては、参照アクセスが直接アドレッシングに置き換えられ、オーバーヘッドが発生しないケースもあります。
実際の挙動を確かめるには、コンパイラにアセンブリを出力させ、参照渡しと値渡しの違いや最適化の有無を比較してみるとより確実だと思います。
There was a problem hiding this comment.
コメントありがとうございます。このあたりのことはよくわかっていないのでもう少し調べてみます。
|
|
||
| numsのヒストグラムをmapで取って、そのmapの要素をpairにして小さい順のpriority_queueに突っ込んでサイズkに保ってindexをvectorにして返すという方針で解きました。 | ||
|
|
||
| ### step2 |
There was a problem hiding this comment.
ほかの方が書かれたソースコードは読まれましたでしょうか？もしまだ読まれていないのであれば、読まれることをお勧めいたします。また、読んだソースコードのリンクと、読んだ際の所感を添えていただくと、レビューワーにとって有益な情報となると思います。
There was a problem hiding this comment.
さらっとしか見ていなかったです。見てみます。
There was a problem hiding this comment.
こちらのコードを参考に、値が大きい順のpriority_queueをsolution2として書き直してみました。こちらの方法の方が値が小さい順のpriority_queueを使う方法よりもややスッキリしているかもしれません。
https://github.com/docto-rin/leetcode/pull/9/files#diff-2d1f664c4bc1102fa070db0b0b8329b974594503435ab3437b253c140ebb78f1R327
4 participants
この問題：https://leetcode.com/problems/top-k-frequent-elements/description/
次に解く問題：https://leetcode.com/problems/find-k-pairs-with-smallest-sums/description/