Justin Hernandez
Justin Hernandez

Reputation: 57

Count vowels in a string with ruby

My objective is to return the number of vowels in a given string. I've tried this code:

def count_vowels(string)
  vowels = ['a', 'e', 'i', 'o', 'u']
  chars_ary = string.split(//)
  ary_with_vowels = chars_ary.take_while {|letter| vowels.include?(letter)}
  return ary_with_vowels.length
end

and it doesn't pass the majority of test cases. I understand that there are multiple other ways to solve this problem, but I want to solve it using the code that I provided.

Can someone give me some insight as to why this isn't working?

Upvotes: 2

Views: 6629

Answers (4)

brff19
brff19

Reputation: 834

Try something like this, its looking at each character in the vowels array and comparing it to each character in the string and the p it if the condition is true.

def vowles(string)
arr = []
vowels = %w(a e i o u)
vowels.each do |x|
    if string.each_char { |letter| (arr << x) if x == letter }
    end
  end
  p "#{arr} has #{arr.count} vowels"
end
vowles("wwwweeeeeee")

or the more efficient

def vowels(string)
  p string.scan(/[a e i o u]/).count
end
vowels("hello world")

Upvotes: 0

Cary Swoveland
Cary Swoveland

Reputation: 110685

Let's benchmark a few approaches.

require 'fruity'
require 'set'

SET_OF_VOWELS = %w| a e i o u |.to_set

def string_count(str)
  str.count('aeiou')
end

def set_include?(str)
  str.each_char.count { |c| SET_OF_VOWELS.include?(c) }
end

def use_hash(str)
  h = str.each_char.with_object(Hash.new(0)) { |c,h| h[c] += 1 }
  SET_OF_VOWELS.sum { |c| h[c] }
end

alpha = ('a'..'z').to_a

[1_000, 10_000, 100_000, 1_000_000].each do |n|

  puts "\nString length = #{n}"     
  str = Array.new(n) { alpha.sample }.join

  compare(
    string_count: -> { string_count(str) },
    set_include?: -> { set_include?(str) },
    use_hash:     -> { use_hash(str) }
  )
end    

The results are as follows.

String length = 1000
Running each test 1024 times. Test will take about 9 seconds.
string_count is faster than set_include? by 159x ± 1.0
set_include? is faster than use_hash by 37.999999999999986% ± 1.0%

String length = 10000
Running each test 128 times. Test will take about 11 seconds.
string_count is faster than set_include? by 234x ± 1.0
set_include? is faster than use_hash by 35.00000000000001% ± 1.0%

String length = 100000
Running each test 16 times. Test will take about 14 seconds.
string_count is faster than set_include? by 246x ± 1.0
set_include? is faster than use_hash by 35.00000000000001% ± 1.0%

String length = 1000000
Running each test 2 times. Test will take about 18 seconds.
string_count is faster than set_include? by 247x ± 1.0
set_include? is faster than use_hash by 34.00000000000001% ± 1.0%

Upvotes: 3

user1934428
user1934428

Reputation: 22237

This way is easier:

 def count_vowels(string)
   string.count('aeiou')
 end

Upvotes: 9

user229044
user229044

Reputation: 239302

take_while is the wrong method here. It starts at the beginning, and "takes" elements as long as the block returns a truthy value. It stops the first time you encounter a letter that isn't a vowel.

You want select which selects all the elements for which the block returns a truthy value.

Upvotes: 5

Related Questions