CODEWITHSUNDEEP
ruby
tcpic94
tcpic94

Reputation: 37

Why is this ruby test failing?

I am doing some of the kata challenges on codewars:

This is the challenge: An isogram is a word that has no repeating letters, consecutive or non-consecutive. Implement a function that determines whether a string that contains only letters is an isogram. Assume the empty string is an isogram. Ignore letter case.

I have the following piece of code:

def is_isogram(string)
  string.downcase!
  ('a'..'z').each do |letter|
    return string.count(letter) <= 1 || string.length == 0 ? true : false
  end
end

with the following tests:

Test.assert_equals(is_isogram("Dermatoglyphics"), true )
Test.assert_equals(is_isogram("isogram"), true )
Test.assert_equals(is_isogram("aba"), false, "same chars may not be adjacent" )
Test.assert_equals(is_isogram("moOse"), false, "same chars may not be same case" )
Test.assert_equals(is_isogram("isIsogram"), false )
Test.assert_equals(is_isogram(""), true, "an empty string is a valid isogram" )

My code fails on the fourth test. Could someone please shed some light on what I am doing wrong and how I can get the test to pass?

Upvotes: 1

Views: 794

Answers (2)

Batz
Batz

Reputation: 372

try this

 def is_isogram(string)
  string.downcase.chars.uniq == string.downcase.chars
 end

Upvotes: 0

sawa
sawa

Reputation: 168249

You are returning from the method at the end of the first iteration. Thus, your code has nothing to do with isogram. Your code will check whether the first character in the range (i.e., "a") is repeated or not.

The first two examples only have one "a", so they return true. The third has more than one "a", so it returns false. The fourth has no "a", so it returns true.

To get your code to work, change it to:

def is_isogram(string)
  string.downcase!
  ('a'..'z').each do |letter|
    return false if string.count(letter) > 1
  end
  return true
end

But a more Rubyish way to write it is:

def is_isogram(string)
  string = string.downcase
  ('a'..'z').none?{|letter| string.count(letter) > 1}
end

Upvotes: 3

Related Questions