Почему мои результаты испортили мои результаты?

Question

Целью этого фрагмента кода (его частью более крупного кода) является определение того, имеет ли год в нем дублирующиеся номера. Вот мой код:

def no_repeat?(year) 
    year = year.to_s 
    string = '' 

    year.each_char{|i| string << year[i] unless string.include?(year[i])} 
    year.length == string.length ? (return false) : (return true) 
    end 

puts no_repeat?(1993) 

Это ВСЕГДА возвращает true, и я не могу понять, почему это происходит. Я попытался расширить тернар в полный оператор if ... по-прежнему возвращает true. Я пытался писать весь этот метод, как цикл While (с двумя индексами, которые сравнивают один индекс к другой)

def no_repeat?(year) 
    year = year.to_s 

i = 0 
while i < year.length 

    i2 = i + 1 
    while i2 < year.length 

     if year[i] == year[i2] 
     return false 
     else 
     return true 
     end 

     i2 += 1 
    end 
    i += 1 
end 

... до сих пор возвращает истину. Я тестировал каждую вещь самостоятельно, и все они отлично работают, пока я не вернусь. Что это касается возвратов? Мне нужны свежие глаза.

Answer 1

То, как вы структурировали тройную версию, неверно. Поскольку ваш метод пытается гарантировать, что ничего не повторяется, он должен возвращать true, когда значение == истинно. Терминал сам предназначен для возврата значения, а не для выполнения выражения типа (return false) внутри его результата. Это работает, но нетрадиционным для того, чтобы быть практически несуществующим.

Тройной должны выглядеть

return year.length == string.length ? true : false 

Что, конечно, может быть упрощено, поскольку выражение == уже возвращает логическое значение.

return year.length == string.length 

Далее, ваше использование year[i] не совсем верно. String#each_char назначает значение символа в i, поэтому вы можете напрямую использовать i. Похоже, что способ, которым вы его использовали, действительно работает, но это не так, как предполагается использовать переменную итератора i.

Это делает ваш метод в:

def no_repeat?(year) 
    year = year.to_s 
    string = '' 

    # i represents the character in this iteration 
    # so you may just directly reference i here 
    year.each_char{|i| string << i unless string.include?(i)} 
    # If the lengths match, return true because there's no repeating character 
    return year.length == string.length 

    # You could omit the 'return' keyword too which is preferred by convention 
    # since Ruby returns the last expression's value implicitly 
    # year.length == string.length 
end 

Answer 2

У вас есть истинные и ложные высказывания перевернутые. В противном случае код работает.

Это работает:

def no_repeat?(year) 
    year = year.to_s 
    string = '' 

    year.each_char{|i| string << year[i] unless string.include?(year[i])} 
    year.length == string.length ? (return true) : (return false) 
end 

no_repeat?(1993) # => false 
no_repeat?(2015) # => true 

Однако есть много способов, которые вы должны улучшить этот код. Ключевое слово return редко используется в Ruby. На самом деле, это совершенно лишний в вашем примере. Эти два метода эквивалентны:

# with `return` 
def no_repeat?(year) 
    year = year.to_s 
    string = '' 

    year.each_char{|i| string << year[i] unless string.include?(year[i])} 
    year.length == string.length ? (return true) : (return false) 
end 

# without `return` 
def no_repeat?(year) 
    year = year.to_s 
    string = '' 

    year.each_char{|i| string << year[i] unless string.include?(year[i])} 
    year.length == string.length 
end 

Во-вторых, с помощью отрицательного («нет») в имени метода делает код трудно следовать. Я предлагаю перевернуть логику и вызвать метод repeat? или даже лучше repeat_chars?.

Наконец, есть более сжатые способы выразить логику, которую вы написали, используя встроенные методы Ruby. Вот одна альтернативная реализация (я уверен, что другие Рубисты могут звонить в с еще более изящные решения):

def repeat_chars?(year) 
    year = year.to_s 
    year.length != year.chars.uniq.length 
end