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

Ranking lines of a file according to number of occurrences

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

Problem

I recently dived into Ruby, so I want to improve this code. Is this good enough?

def ranksystem
    logfile = IO.readlines("some_logfile.log")
    logfile.each do |value|
      value.gsub!(/[\n]+/, "")
    end
    logfile.delete("")

    logcount = Hash.new(0)

    logfile.each do |v|
      logcount[v] += 1
    end

    logarray = logcount.sort_by {|k,v| v}.reverse

    finalreturn = ""

    logarray.each_with_index do |value, index|
      finalreturn = finalreturn + "Rank #{index+1} : #{value[1]} (#{value[0]})\n"
    end

    finalreturn

end


This code reads each line in log file like this:

apple
orange
apple
apple
apple
apple
orange
orange
banana
banana


and converts like this:

Rank 1 : 4 (apple)
Rank 2 : 3 (orange)
Rank 3 : 2 (banana)

Solution

"Cool" enough? It seems fine, if procedural. Depends what you mean by cool.

I might combine a few steps. I'd also refactor and not put everything into one method, since each separate "chunk" does very distinct, and different, things. Separation makes testing easier.

The below isn't necessarily any better (and the chunk that creates the report may actually be worse--I'm not sure; it'd be less efficient), but it provides some alternate avenues to explore further.

def get_log_lines name
  logfile = IO.readlines(name)
  logfile.collect { |l| l.gsub(/\n+/, "")}.reject { |l| l == "" }
end

def ranksystem
  loglines = get_log_lines "some_logfile.log"
  logcount = Hash[loglines.group_by {|l| l}.collect {|k, v| [v.size, k]}]
  i = 0
  logcount.keys.sort.reverse.collect { |n| i += 1; "Rank #{i}: #{n} (#{logcount[n]})" }.join("\n")
end

Code Snippets

def get_log_lines name
  logfile = IO.readlines(name)
  logfile.collect { |l| l.gsub(/\n+/, "")}.reject { |l| l == "" }
end

def ranksystem
  loglines = get_log_lines "some_logfile.log"
  logcount = Hash[loglines.group_by {|l| l}.collect {|k, v| [v.size, k]}]
  i = 0
  logcount.keys.sort.reverse.collect { |n| i += 1; "Rank #{i}: #{n} (#{logcount[n]})" }.join("\n")
end

Context

StackExchange Code Review Q#7774, answer score: 4

Revisions (0)

No revisions yet.