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

Most common letter in string

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

Problem

Problem: Write a method that takes in a string. Your method should return the most common letter in the array, and a count of how many
times it appears.

Note: I solved it with Ruby, but the solution will be almost the same with other OOP language which have arrays and hashes.

Solution:

-
Split the string into an array of its characters/letters.

-
Put each char in a hash as key and starting value = 1.

-
When you find a character again, increase its value by 1.

-
Iterate your hash to find the most common character.

My solution works, but, is there a simpler & better way to do it?

What are the disadvantages of my solution?

def most_common_letter(string)
    hash = {}
    arr = string.split("")
    i = 0
    ge = "" # the char which occurs the most times in string
    max = 0 # number of times ge occurs in string
    while i  max)
            max = v
            ge = k
        end
    end
    return ge.to_s + ":" + max.to_s
end

#Test
puts most_common_letter("abca")
puts most_common_letter("g")

Solution

while i < arr.length


In your loop, we are only accessing the current element and not doing any index arithmetic. Use arr.each{}.

if hash.has_key? e
        hash[e] += 1
    else
        hash[e] = 1
    end


When we encounter a key, we just want to increment by 1. If a key doesn't exist, use the default value and increment by 1. You can initialize your hash to have a default value of 0 by hash = Hash.new(0).

# Find most common character
hash.each do |k,v|
    if(v > max)
        max = v
        ge = k
    end
end


This can simply be written using hash.max_by{} and it does the work for you of returning a key-value pair.

Putting it all together, you get

def most_common_letter(string)
  hash = Hash.new(0)
  string.split('').each{ |ch| hash[ch] += 1 }
  hash.max_by{ |ch,count| count }
end


And if you want to clean it up so its a single chained statement.

def most_common_letter(string)
  string.split('')
        .each_with_object(Hash.new(0)) { |ch,hash| hash[ch] += 1 }
        .max_by{ |ch,count| count }
end



What are the disadvantages of my solution ?

The biggest issue is that it only returns 1 letter/count pair. What happens if you call most_common_letter("aabb"). Your function returns the pair that is first encountered that sets max {'a', 2} rather than {{'a', 2}, {'b', 2}}. If you want to return all of the max pairs, build your count hash, find the max value, then return all elements from your hash that has that max value using Hash#select{}.

Code Snippets

while i < arr.length
if hash.has_key? e
        hash[e] += 1
    else
        hash[e] = 1
    end
# Find most common character
hash.each do |k,v|
    if(v > max)
        max = v
        ge = k
    end
end
def most_common_letter(string)
  hash = Hash.new(0)
  string.split('').each{ |ch| hash[ch] += 1 }
  hash.max_by{ |ch,count| count }
end
def most_common_letter(string)
  string.split('')
        .each_with_object(Hash.new(0)) { |ch,hash| hash[ch] += 1 }
        .max_by{ |ch,count| count }
end

Context

StackExchange Code Review Q#95914, answer score: 6

Revisions (0)

No revisions yet.