Most common letter in string 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 &amp; 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 &lt; arr.length
        e = arr[i]
        if hash.has_key? e
            hash[e] += 1
        else
            hash[e] = 1
        end
        i = i + 1
    end

    # Find most common character
    hash.each do |k,v|
        if(v &gt; 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")
while i &lt; 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 &gt; 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{}.