Reputation: 57
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
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
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
Reputation: 22237
This way is easier:
def count_vowels(string)
string.count('aeiou')
end
Upvotes: 9
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