ChrisWilson ChrisWilson - 7 months ago 32
Ruby Question

Ruby: How to create a letter counting function

I wouldn't have asked for help without first spending a few hours trying to figure out my error but I'm at a wall. So there is my attempt but I'm getting false when I try to pass the argument though and I'm not sure why. I know there are other ways to solve this problem that are a little shorter but I'm more interested in trying to get my code to work. Any help is much appreciated.

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.

def most_common_letter(string)
idx1 = 0
idx2 = 0
counter1 = 0
counter2 = 0

while idx1 < string.length
idx2 = 0
while idx2 < string.length
if string[idx1] == string[idx2]
counter1 += 1
end
idx2 += 1
end
if counter1 > counter2
counter2 = counter1
counter1 = 0
var = [string[idx1], counter2]
end
idx1 += 1
end
return var
end

puts("most_common_letter(\"abca\") == [\"a\", 2]: #{most_common_letter("abca") == ["a", 2]}")
puts("most_common_letter(\"abbab\") == [\"b\", 3]: #{most_common_letter("abbab") == ["b", 3]}")

Answer

I didn't rewrite your code because I think it's important to point out what is wrong with the existing code that you wrote (especially since you're familiar with it). That said, there are much more 'ruby-like' ways to go about this.

The issue

counter1 is only being reset if you've found a 'new highest'. You need to reset it regardless of whether or not a new highest number has been found:

def most_common_letter(string)
  idx1 = 0
  idx2 = 0
  counter1 = 0
  counter2 = 0
  while idx1 < string.length
    idx2 = 0
    while idx2 < string.length
      if string[idx1] == string[idx2]
        counter1 += 1
      end
      idx2 += 1
    end
    if counter1 > counter2
      counter2 = counter1 
      # counter1 = 0  THIS IS THE ISSUE
      var = [string[idx1], counter2] 
    end
    counter1 = 0  # this is what needs to be reset each time
    idx1 += 1
  end
  return var
end

Here's what the output is:

stackoverflow master % ruby letter-count.rb
most_common_letter("abca") == ["a", 2]: true
most_common_letter("abbab") == ["b", 3]: true

I think you're aware there are way better ways to do this but frankly the best way to debug this is with a piece of paper. "Ok counter1 is now 1, indx2 is back to zero", etc. That will help you keep track.

Another bit of advice, counter1 and counter2 are not very good variable names. I didn't realize what you were using them for initially and that should never be the case, it should be named something like current_count highest_known_count or something like that.

Comments