703_Kth_Largest_Element_in_a_stream追加 by haniwachann · Pull Request #1 · haniwachann/leetcode

haniwachann · 2024-11-08T13:56:30Z

<問題>
https://leetcode.com/problems/kth-largest-element-in-a-stream/

haniwachann · 2024-11-08T19:51:56Z

初めての投稿です。
Cs初心者なので、コーディング、調べ方、勉強の仕方など誤りがあったら教えていただけると嬉しいです。
よろしくお願いします。

oda · 2024-11-08T21:58:41Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+    vector<int> scores;
+    int kth;


外からアクセスが想定されていないものは private に。

以前野田さんにおすすめされたのですが、「effective c++」にこの辺りのことが記載されておりますので読んでみるといいと思いました。

oda · 2024-11-08T21:58:47Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+        scores=nums;
+        sort(scores.rbegin(),scores.rend());
+        kth=k;


スペースの空け方のルールはなにか統一的なものがあればなんでもいいと思います。
Google Style Guide は一つです。
https://google.github.io/styleguide/cppguide.html

ありがとうございます。
リンク参考にさせていただきます。
違和感を持たれないようにコードを書くことを心がけます。

oda · 2024-11-08T21:59:29Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+        if((int)size(scores)>kth){
+            return scores[kth-1];    
+        }else{
+            return scores[kth-1];
+        }


これ、条件分岐して同じ式ですね。それだったらまとめていいですが、なんか意図があったんじゃないでしょうか。全体の大きさを小さくするとか。kth より多いときには減らせますね。

(int) もこの場合は不要ですかね。

すみません。
vectorの要素数がkthよりも大きいことが前提ならば必要ないですが、vectorの要素数がkthより小さいときはエラーを知らせるように書こうとしてました。

教えていただいた通り、vectorの長さを減らすようにします！

oda · 2024-11-08T21:59:33Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+            if(canditates.count(nums[i])!=0){
+                canditates[nums[i]]++;
+            }else{
+                canditates[nums[i]]=1;


ない場合でも、アクセスするだけで作られて0になります。

アクセスするだけで要素が作られること知りませんでした...
そのような知識はリファレンスを日頃から読んで少しづつ身につけていくのがよいでしょうか。

https://cpprefjp.github.io/reference/map/map/op_at.html
「対応する要素が存在しない場合は、要素を値初期化して参照を返す。」
そうですね。とりあえず、読みましょう。
だから canditates[nums[i]]++; だけでいいですね。

oda · 2024-11-08T21:59:47Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+            if(canditates.begin()->second==1){
+                canditates.erase(canditates.begin());
+            }else{
+                canditates.begin()->second--;
+            }


とりあえず、1を引いて、0以下になっていたら消すという方法もあります。

ありがとうございます。
条件分岐のelseが1つ減ってわかりやすくなりそうです。

oda · 2024-11-08T22:02:15Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+        auto iter=canditates.begin();
+        int ans= iter->first;
+        return ans;


これくらいならば一行でもいいでしょう。

SuperHotDogCat · 2024-11-09T17:04:54Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+            }
+        }
+
+        for(int i=k;i<(int) size(nums);i++){


なんだかfor, if, else文のスペースの開け方に違和感を感じてしまいます.
https://ttsuki.github.io/styleguide/cppguide.ja.html#Formatting_Looping_Branching
こちらにGoogle Coding Style Guideのifのスペースの開け方があります

違和感を教えていただきありがとうございます。
いただいたリンクに目を通して、少しずつ修正していきます！

SuperHotDogCat · 2024-11-09T17:05:52Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+
+今度は、時間を測りながら、もう一回、書きましょう。書いてアクセプトされたら文字消してもう一回書く。これを10分以内に一回もエラーを出さずに書ける状態になるまで続ける。3回続けてそれができたらその問題はOK。
+
+実施しました。


他の人はstep3も載せているので載せちゃうといいと思います

haniwachann · 2024-11-10T07:39:24Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md


-実施しました。
+実施しました。
+


step4として、レビューを受けてコードを修正いたしました。

nodchip · 2024-11-10T08:05:55Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+class KthLargest {
+public:
+
+    map<int,int> canditates;


multimap を使っても同様の処理が書けると思います。余裕があれば挑戦してみてください。

ありがとうございます。
multimapを使用する場合は、keyを問題で与えられた値として、keyの値がmultimap全体の中で何個目であるかをvalueに入れるイメージでしょうか。

いえ、 multipmap の要素数が k を超えるたびに、一番小さい値を削除していくことになると思います。

multimapではkey,valueともに同じものが複数存在できるという理解でいいでしょうか。
https://cpprefjp.github.io/reference/map/multimap.html

しまった…。書き間違えました…。 multiset のほうでした…。

multiset教えていただきありがとうございます！
実装のイメージつきました！

Ryotaro25 · 2024-11-10T09:06:20Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+class KthLargest {
+public:
+
+    map<int,int> canditates;


こちらはcandidateの誤りでしょうか？

スペルミスです🙏

Ryotaro25 · 2024-11-10T09:08:04Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+        }
+
+        auto iter=canditates.begin();
+        int ans= iter->first;


ansだと何が格納されているのか分からないので、変数名から何が入っているのか分かるような変数名がいいと思いました。

ありがとうございます。
kth_numとかがいいかと思いました。

liquo-rice · 2024-11-10T09:35:07Z

03_Heap,PriorityQueue/703_Kth_Largest_Element_in_a_stream.md

+            canditates[nums[i]]++;
+            canditates.begin()->second--;
+            if (canditates.begin()->second<=0) {
+                canditates.erase(canditates.begin());
+            }


L185-189とほとんど同じです。関数に切り出せますか？

ありがとうございます。
privateに関数を追加して処理をまとめました。

class KthLargest { private: map<int,int> candidates; int kth; void add_num_to_candidates(int nums_add){ candidates[nums_add]++; candidates.begin()->second--; if (candidates.begin()->second<=0) { candidates.erase(candidates.begin()); } } public: KthLargest(int k, vector<int>& nums) { kth=k; for (int i=0; i<k; i++) candidates[nums[i]]++; for (int i=k; i<size(nums); i++) add_num_to_candidates(nums[i]); } int add(int val) { add_num_to_candidates(val); return candidates.begin()->first; } };

下のテストケースがパスしないようです。

["KthLargest","add","add","add","add","add"] [[1,[]],[-3],[-2],[-4],[0],[4]]

初期配列が空だと
for (int i=0; i<k; i++) candidates[nums[i]]++;
のnums[i]で存在しない要素を呼び出していて、エラーが出るようでした。
for文の実行条件をi<k && i<size(nums)に修正しました。

また、add_num_to_candidatesの中で、配列の数がkthに満たない場合も
加えた値を削除してしまっていたので、candidates_counterの変数を用意して、
要素数がkthより大きい時のみ、削除する処理をするように条件分けしました。

class KthLargest { private: map<int,int> candidates; int candidates_counter=0; int kth; void add_num_to_candidates(int nums_add){ candidates[nums_add]++; candidates_counter++; if (candidates_counter>kth) { candidates.begin()->second--; if (candidates.begin()->second<=0) { candidates.erase(candidates.begin()); } } } public: KthLargest(int k, vector<int>& nums) { kth=k; for (int i=0; i<k && i<size(nums); i++){ candidates[nums[i]]++; candidates_counter++; } for (int i=k; i<size(nums); i++) add_num_to_candidates(nums[i]); } int add(int val) { add_num_to_candidates(val); return candidates.begin()->first; } };

一回、自然言語にしてみるんですが、重複している感じがしませんか。
kth は、「この KthLargest のインスタンスがいくつまで管理するか」
candidates は、「大きいものから k 個(追加された数が k に満たない場合はすべて)の分布」
candidates_counter は、「消去したものも含めて追加された数」ですね。

add_num_to_candidates は、意味は「数字を追加して candidates の分布を更新」、操作としては「数字を追加し、k を超えていたら一番小さいのを消す」
KthLargest は、意味は「k と nums で初期化」、操作としては「入力の配列に対して、k 番目までは candidates を更新し、それ以降 add_num_to_candidates を呼ぶ」
add は、意味は「数字を追加して、k 番目に大きいものを返す」、操作としては「add_num_to_candidates を呼んで、k 番目を返す」

個人的には
KthLargest は add_num_to_candidates を nums に対して呼ぶのでよいでしょう。
add_num_to_candidates と add は一緒にしてしまうかもしれません。
candidates_counter は、意味を「中に入っている個数」にして消したら減らしたいです。

変数、関数ともに自然言語にしてみると、
・addがadd_num_to_candidatesを呼んでるだけになっている。
・KthLargestのk番目とそれ以降で分けていることを、すでにadd_num_to_candidatesで処置している。
ことがよくわかります...

candidates_counterの変数の意味は中に入っている個数という意味で使いたかったのですが、
実装上は違う意味になっていました...
コードを頭の中でミスなく動かす練習をしていきたいです。

以下の通り直してみました。

class KthLargest { private: map<int,int> candidates; int candidates_counter=0; int kth; public: KthLargest(int k, vector<int>& nums) { kth=k; for (int i=0; i<size(nums); i++)　add(nums[i]); } int add(int val) { candidates[val]++; candidates_counter++; if (candidates_counter>kth) { candidates.begin()->second--; if (candidates.begin()->second<=0) { candidates.erase(candidates.begin()); } candidates_counter--; } return candidates.begin()->first; } };

いいかと思います。
削除の際のcandidates.begin()は三回繰り返しているので変数においてもいいかもしれませんね。
他に気になるのは、スタイル(演算子の前後にスペースをいれるか、ぶら下がり for を使うか、インクリメント・デクリメントは前置が好きか、など)ですが、趣味の範囲です。

703_Kth_Largest_Element_in_a_stream追加

991655b

oda reviewed Nov 8, 2024

View reviewed changes

SuperHotDogCat reviewed Nov 9, 2024

View reviewed changes

＃step4としてコードを修正

16cb7a2

haniwachann commented Nov 10, 2024

View reviewed changes

nodchip reviewed Nov 10, 2024

View reviewed changes

Ryotaro25 reviewed Nov 10, 2024

View reviewed changes

liquo-rice reviewed Nov 10, 2024

View reviewed changes

haniwachann added 2 commits November 10, 2024 08:04

step4_2 関数に処理をまとめた

7cec978

step4_3 初期配列空の時の対処、配列要素数がkth以下の時の処理を修正

72cb294


		今度は、時間を測りながら、もう一回、書きましょう。書いてアクセプトされたら文字消してもう一回書く。これを10分以内に一回もエラーを出さずに書ける状態になるまで続ける。3回続けてそれができたらその問題はOK。

		実施しました。 No newline at end of file


		実施しました。 No newline at end of file
		実施しました。

Conversation

haniwachann commented Nov 8, 2024

Uh oh!

haniwachann commented Nov 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haniwachann Nov 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

haniwachann Nov 9, 2024 •

edited

Loading