Skip to content

347. Top k frequent elements#9

Open
dxxsxsxkx wants to merge 2 commits into703_kth_largest_elementfrom
347_top_k_frequent_elements
Open

347. Top k frequent elements#9
dxxsxsxkx wants to merge 2 commits into703_kth_largest_elementfrom
347_top_k_frequent_elements

Conversation

@dxxsxsxkx
Copy link
Copy Markdown
Owner

value_count[num]++;
}

for (auto &[value, count] : value_count) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらのコメントをご参照ください。
hemispherium/LeetCode_Arai60#10 (comment)

public:
vector<int> topKFrequent(vector<int>& nums, int k) {
map<int, int> value_count;
map<int, vector<int>, greater<int>> count_value;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

変数を使用箇所から離れた場所で定義すると、読み手の視点では、定義した場所から使用箇所まで変数の内容を覚えておかなければならなかったり、使用箇所を読んでいるときに変数の定義を読み返すときに、目線を大きく上下に動かさなければならず、負荷になると思います。変数は使用する直前まで定義を遅らせることをお勧めいたします。

}

vector<int> top_k_frequent;
while (count_and_value.size() != 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while (!count_and_value.empty()) {

のほうがシンプルだと思います。

public:
vector<int> topKFrequent(vector<int>& nums, int k) {
map<int, int> value_to_count;
map<int, vector<int>, greater<int>> count_to_value;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

値が value の配列のため、 count_to_values としても良いと思います。

## コード

元の方針のまま、名付けやループのロジック、初期化のタイミングを変えることにした。
あと、`#include <...>` をちゃんと書くことにした。
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

ファイルとしてコピペしてそのままコンパイルが通る状態に持っていくには、vectorやmapの前にstd::をつけるといいかもしれませんね。e.g., vector<int> -> std::vector<int>

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そちらも大事ですね。ありがとうございます。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using namespace std は、ヘッダーではほぼ間違いなくしないですね。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.69hbsczcvb03

map<int, vector<int>, greater<int>> value_to_count;

for (int num : nums) {
count_to_value[num]++;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count_to_valuevalue_to_countが入れ替わっている気がします。
ここは、keyがnumでvalueが出現回数なので num_to_count (value_to_count)かと思います。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本当ですね。指摘ありがとうございます。

public:
vector<int> topKFrequent(vector<int>& nums, int k) {
map<int, int> count_to_value;
map<int, vector<int>, greater<int>> value_to_count;
Copy link
Copy Markdown

@huyfififi huyfififi Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

前回の問題 (Kth Largest Element in a Stream) との関連と、問題文に

Follow up: Your algorithm's time complexity must be better than O(n log n), where n is the array's size.

とあることを考えると、heap / priority queue を練習してみるのもいいと思います。

mapが自動でソートしてくれることを利用した解法かと思いますが、ソートを用いてもいいのなら

import collections


class Solution:
    def topKFrequent(self, nums: list[int], k: int) -> list[int]:
        num_to_count = collections.defaultdict(int)
        for num in nums:
            num_to_count[num] += 1

        count_and_nums = [(count, num) for num, count in num_to_count.items()]
        count_and_nums.sort(reverse=True)

        return [num for _, num in count_and_nums[:k]]

みたいな解法の方が素直かと思います。(Pythonですみません、C++に不慣れなもので)

value_to_count[num]++;
}

for (auto &[value, count] : value_count) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: こちら、typoかと思います。

Suggested change
for (auto &[value, count] : value_count) {
for (auto &[value, count] : value_to_count) {


for (int value : it->second) {
top_k_frequent.push_back(value);
k--;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

以下のような入力の時に、top_k_frequent のサイズが k よりも大きくなるような気がします。

入力:

  • k = 1
  • nums = [1, 1, 2, 2]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、すいません、leetcode の制約には答えは一意になると書いてあるので、このような入力は来ないという前提なんですね

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これはいい指摘です。実際のユースケースでは、例えば、ウェブサイトでアクセス元トップ10とかでしょう。実際には制約を気がついたら破られそうですね。ページネーションなどとの兼ね合いも気になります。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants