Правонарушение Rubocop: используйте возврат условного выражения для назначения и сравнения переменных. - PullRequest
Новая
       25

Правонарушение Rubocop: используйте возврат условного выражения для назначения и сравнения переменных.

2 голосов
/ 22 апреля 2019

Мое нарушение Рубокопа говорит мне, что мне нужно «Использовать возврат условного выражения для назначения и сравнения переменных»

Пока я пытался это исправить, меня обидело, что моя «строка метода слишком длинная».

Я пробовал рефакторинг на другой метод, но мой код сломался.

Как мне сократить или изменить этот код? 

HSH = { 'a' => 'z', 'b' => 'y', 'c' => 'x', 'd' => 'w', 'e' => 'v', \
        'f' => 'u', 'g' => 't', 'h' => 's', \
        'i' => 'r', 'j' => 'q', 'k' => 'p', 'l' => 'o', 'm' => 'n' }.freeze


def encoder(str)
  encoded_string = ''
  str.chars.each do |char|
    encoded_string = if HSH.key?(char) then encoded_string += HSH[char]
                     elsif HSH.invert.key?(char) then encoded_string += HSH.invert[char]
                     else encoded_string += char
                     end
  end
  encoded_string
end

Когда я запустил свой тестовый набор, все было в порядке.

Но преступление из-за рубокопа дало мне слишком длинную строку метода.

Ответы [ 6 ]

4 голосов
/ 22 апреля 2019

Нет хэша: 

ALPHABET = ("a".."z").to_a.join

def encoder(str)
  str.tr(ALPHABET, ALPHABET.reverse)
end
2 голосов
/ 22 апреля 2019 
HSH = {
  'a' => 'z', 'b' => 'y', 'c' => 'x',
  'd' => 'w', 'e' => 'v', 'f' => 'u',
  'g' => 't', 'h' => 's', 'i' => 'r',
  'j' => 'q', 'k' => 'p', 'l' => 'o',
  'm' => 'n'
}.freeze

def encoder(str)
  str.chars.map { |char| HSH[char] || HSH.invert[char] || char }.join
end
1 голос
/ 22 апреля 2019

Некоторые из нас предложили более эффективные способы реализации вашего encoder метода.Но все мы (включая меня) на самом деле не ответили на ваш вопрос или не увидели главную проблему в вашем коде:

Мое правонарушение по Рубокопу говорит мне, что мне нужно «использовать возврат условногодля присваивания и сравнения переменных '

Пока я пытался исправить это, я получил еще одно оскорбление из-за того, что моя "строка метода слишком длинная".совет ... вы присвоили условному результату значение ... но я думаю, что вы пропустили оценку.Я даже не уверен, что это условный результат, о котором он говорил.Я предполагаю, что вы добавили назначение encoded_string = ....

Это назначение бесполезно, так как вы уже добавили символ к encoded_string внутри блоков if.Вам не нужно назначать его снова.

Если перефразировать то, что, как я предполагаю, было кодом вашей версии 1, вот более эффективный способ следовать советам Рубокопа.Не делайте назначения внутри каждого условия ... делайте только одно назначение с результатом условий: 

encoded_string += if HSH.key?(char) then HSH[char]
                  elsif HSH.invert.key?(char) then HSH.invert[char]
                  else char
                  end

Это заканчивается меньшим количеством кода и соответствует вашему оригинальному стилю и подходу кодирования.Это может даже сделать Рубокопа счастливым.Следующим шагом к созданию прекрасного кода будет устранение излишних key? тестов: 

encoded_string += if HSH[char] then HSH[char]
                  elsif HSH.invert[char] then HSH.invert[char]
                  else char
                  end

Оттуда это небольшой шаг по устранению блоков if/elsif с ||.И пока мы это делаем, мы изменим += на <<, чтобы избежать "создания миллиарда промежуточных ненужных экземпляров String".(Спасибо за предложение, @Aleksei Matiushkin) 

encoded_string << HSH[char] || HSH.invert[char] || char

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

1 голос
/ 22 апреля 2019

Внимание!Не используйте этот ответ! правильный путь предоставлен @steenslag здесь .

Используйте хеш со всеми буквами, явно отображенными и процедура по умолчанию: 

HSH =
  (?a..?z).zip((?a..?z).to_a.reverse).to_h.
  tap { |h| h.default_proc = ->(_, k) { k }}.
  freeze

def encoder(str)
  str.chars.map(&HSH.method(:[])).join
end
0 голосов
/ 22 апреля 2019

Как и в случае ответа @ Steenslag, нет необходимости преобразовывать строку в массив, отображать каждый элемент массива и объединять результат обратно в строку. 

def encode_decode(str)
  str.gsub(/./) { |c| ('a'..'z').cover?(c.ord) ? (219-c.ord).chr : c }
end

plain_text = "The launch code is 'Bal3De8Rd0asH'."
  #=> "Tsv ozfmxs xlwv rh 'Bzo3Dv8Rw0zhH'." 
encode_decode(coded_text)
  #=> "The launch code is 'Bal3De8Rd0asH'."
0 голосов
/ 22 апреля 2019

Я бы пошел дальше и расширил ваш хеш до всех 26 букв, чтобы вы могли избежать обратного поиска. Это упростит ваш код, удалив один случай, что может убить Rubocop ... Но что более важно, вы будете использовать хеш-индекс для большей эффективности и производительности. Обратный поиск в хэше дорогой, так как он должен читать (до) каждое значение.

Рассмотрим кодировку «1 + 2». Она выполнит три быстрых сканирования индекса, а затем три полных сканирования массива, только чтобы вернуть исходную строку.

С полностью заполненным хешем потребуется всего три быстрых сканирования.

Вот ваш оригинальный код с минимальными изменениями для достижения вашей цели: (Есть более короткие способы сделать это (подсказка: tr или map), но более короткий не так важен, как простой и удобный для программиста использование код.) 

translation = { 
  'a' => 'z', 'b' => 'y', 'c' => 'x', 'd' => 'w', 'e' => 'v', 'f' => 'u', 'g' => 't', 
  'h' => 's', 'i' => 'r', 'j' => 'q', 'k' => 'p', 'l' => 'o', 'm' => 'n', 'n' => 'm', 
  'o' => 'l', 'p' => 'k', 'q' => 'j', 'r' => 'i', 's' => 'h', 't' => 'g', 'u' => 'f', 
  'v' => 'e', 'w' => 'd', 'x' => 'c', 'y' => 'b', 'z' => 'a'
}.freeze

def encoder(str)
  encoded_string = ''

  str.chars.each do |char|
    encoded_string << translation[char] || char
  end
  encoded_string
end

Вы можете даже рассмотреть вопрос о расширении хеша до прописных и строчных букв или даже до всех 256-значных значений, в зависимости от решаемой проблемы. Но давайте согласимся игнорировать символы Unicode!

Назад к Rubocop ... Самое простое, надежное решение для любого вида «слишком длинных / слишком сложных» предупреждений - вытащить код для нового метода. Напишите def charswap и используйте это как тело вашего цикла. Это облегчит написание тестов для загрузки. Но, расширив массив перевода до всех 26 букв, код становится настолько простым, что рефакторинг на самом деле не нужен.

...