Cannot simplify `if` statement condition

Question

Here is a function to count in an if statement the vowels contained in a string:

 def count_vowels(string)
 sum = 0
  n = 0
  while n < string.length
    if string[n] == "a"||string[n]=="i"||string[n]=="u"||string[n]=="e"||string[n]=="o"
      sum += 1
    end
    n+=1
  end
  return sum
end

I found the repetitive string[n] == being redundant and replaced it with:

if string[n] == ("a"||"i"||"u"||"e"||"o")

However, in this code, the function does not return the correct counts. Why does the simplified if statement not work here?

possible duplicate of [Test whether a variable equals either one of two values](http://stackoverflow.com/questions/2196414/test-whether-a-variable-equals-either-one-of-two-values) — toro2k, Apr 24 '15 at 15:07
@Stefan shows you how to replace all the code in your method with one 21-character line. Isn't Ruby wonderful? — Cary Swoveland, Apr 24 '15 at 21:56

score 4 · Accepted Answer · answered Apr 24 '15 at 14:36

4

It doesn't work, because a == (x || y) is not expanded to a == x || a == y.

Instead, a is compared against the result of (x || y).

if string[n] == ("a"||"i"||"u"||"e"||"o")

is equivalent to:

if string[n] == "a"

because:

("a"||"i"||"u"||"e"||"o") #=> "a"

If you want to simplify your code, use count:

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

answered Apr 24 '15 at 14:36

Stefan

109,145
14
143
218

You should probably mention in the answer that it won't cater for capital letters. This might answer the question, but the OP probably wants to `count_vowels` which should probably be lower and upper case vowels. – Ryan-Neal Mes Apr 25 '15 at 12:51

score 2 · Answer 2 · edited Apr 25 '15 at 13:23

2

To answer your question

string[n] == ("a"||"i"||"u"||"e"||"o")

This part ("a"||"i"||"u"||"e"||"o") would always evaluate to "a"

so you are essentially writing

string[n] == "a"

A better way to do this might be

def count_vowels(my_string)
  mystring.chars.count{ |c| c  =~ /[aeiou]+/i }
end

You could also extend the string class for fun

class String
  def vowels_count
    chars.count{ |c| c  =~ /[aeiou]+/i }
  end
end

edited Apr 25 '15 at 13:23

Wayne Conrad

103,207
26
155
191

answered Apr 24 '15 at 14:38

Ryan-Neal Mes

6,003
7
52
77

1

Or even better: `'foobar'.count('aeiou') #=> 3`. – toro2k Apr 24 '15 at 14:44
OP = Original Poster, the person who wrote the question. – Stefan Apr 24 '15 at 15:02

score 1 · Answer 3 · answered Apr 24 '15 at 14:36

There are various ways you can do what you want but the reason it doesn't work is because ("a"||"e"||"i"||"o"||"u") is evaluated by ruby to return the first of those characters that isn't false or nil. Essentially that clause always returns "a":

2.2.1 :001 > ("a"||"e"||"i"||"o"||"u")
=> "a"

This means that you are always testing if string[n] == "a" which is clearly not what you are aiming to achieve.

potashin · Answer 4 · 2015-04-24T14:49:10.720

0

It doesn't return correct count because || in if string[n] == ("a"||"i"||"u"||"e"||"o") evaluates to the first non-false condition, in this case, "a" (it is not nil or false), so basically it is the same as if string[n] == "a".You can try with include? :

%w( a i u e o).include? string[n]

or in?:

string[n].in? %w( a i u e o)

edited Apr 24 '15 at 14:49

answered Apr 24 '15 at 14:12

potashin

44,205
11
83
107

Nermin · Answer 5 · 2015-04-24T14:27:33.567

0

Add regular expersion

 def count_vowels(string)
 sum = 0
  n = 0
  while n < string.length
    if string[n] =~ /[aiueo]/ # will match a, i, u, e or o
      sum += 1
    end
    n+=1
  end
  return sum
end

You can even more simplify your code

def count_vowels(string)
  string.split('').select{|char| char =~ /[aeiou]/}.length
end

will do the same functionality

  string.split('')

will give you array of characters

.select{|char| char =~ /[aeiou]/}

will select only 'a' 'e' 'i' 'o' 'u' characters

.length

will count number of those characters

edited Apr 24 '15 at 14:27

answered Apr 24 '15 at 14:12

Nermin

6,118
13
23

2

This is not answering the actual question. – Ryan-Neal Mes Apr 24 '15 at 14:45

score 0 · Answer 6 · answered Apr 24 '15 at 14:38

Your if statement doesn't work because == ("a" || ...) doesn't tell Ruby to check if it's equal to any of these. Instead, it evaluates ("a" || ...) and checks if string[n] is equal to that.

The "proper" simplified expression would be this:

string[n] =~ /aeiou/i

Assuming you want case-insensitivity, that is. If not, use this:

string[n] =~ /aeiou/

Note that with this solution you'll have to escape any regex metacharacters (., [], (), etc.)

If you don't want to do that, use something like this instead:

['a', 'e', 'i', 'o', 'u'].include? string[n]

Cannot simplify `if` statement condition

6 Answers6