HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Slowly working word counter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wordslowlyworkingcounter

Problem

I have a problem where, given N terms, the task is to find the k most frequent terms from given N terms. My code is correct, but I have a time limit 10 seconds and my code doesn't pass.

Constraints:

  • \$0



  • \$0



N = gets.to_i

if N  a.last) ? 1 : (b.last  b.first)) }

  i = 0
  for key, val in s_l
    puts key if i < k
    i += 1
  end
end


And how can I print only key of hash? Currently I use for key, val in hash.

Solution

The solution is slow because you aren't using data structures effectively. In particular, the nested (0..size).each loops will cause it to take \$O(N^2)\$ time, which will be very problematic for large \$N\$.

You created new_array (which is actually a hash, not an array as the name suggests), but populate it the hard way. The easy way is to loop once through all of the words; for each word you encounter, increment its count by 1. There is no need to construct the intermediate array a.

# Read N, then count occurrences of the next N lines of text
word_counter = Hash.new { |count, word| count[word] = 0 }
n_lines = gets.to_i
n_lines.times { word_counter[gets] += 1 }

# Read k; limit it to the number of distinct words
k = gets.to_i
k = [k, word_counter.keys.length].min

# Sort words by descending frequency, breaking ties by alphebetical order
words_desc_freq = word_counter.sort do |a, b|
  cmp = a.last - b.last                    # .last is the count
  cmp != 0 ? -cmp : (a.first  b.first)  # .first is the word
end

# Output k most frequent words
(0...k).each do |i|
  puts words_desc_freq[i].first
end


The solution above has complexity \$O(N \log N)\$, dominated by the time it takes to sort.

Usually, when constraints are given for problems like this, it means that your program is not expected to handle input exceeding the stated limits gracefully. It does not mean that your program needs to actively reject input that exceeds those limits. In any case, your "validation" isn't quite right: in Ruby, 0 is treated as a true value.

Your code is formatted reasonably well. I would just note that the line that performs the sort is long and should be broken up. One-line blocks are typically written using braces, like (0..size).each { |j| count += 1 if a[i] == a[j] }

Code Snippets

# Read N, then count occurrences of the next N lines of text
word_counter = Hash.new { |count, word| count[word] = 0 }
n_lines = gets.to_i
n_lines.times { word_counter[gets] += 1 }

# Read k; limit it to the number of distinct words
k = gets.to_i
k = [k, word_counter.keys.length].min

# Sort words by descending frequency, breaking ties by alphebetical order
words_desc_freq = word_counter.sort do |a, b|
  cmp = a.last - b.last                    # .last is the count
  cmp != 0 ? -cmp : (a.first <=> b.first)  # .first is the word
end

# Output k most frequent words
(0...k).each do |i|
  puts words_desc_freq[i].first
end

Context

StackExchange Code Review Q#69684, answer score: 2

Revisions (0)

No revisions yet.