Negative Conditional Statement in Ruby

Question

It seems like the RubyMine IDE signals a warning whenever it sees a negative conditional statement. I am wondering why using negative conditional statement is bad? Is it just purely due to readability?

For example, in this code:

class Complement
    def self.of_dna dna_strand
      dna_array =  dna_strand.chars
      dna_complement  = ['']
      dna_structure = ['C', 'G', 'T', 'A']

      dna_array.each do |strand|
        unless dna_structure.include? strand 
          return ''
        end

        case strand
        when "C"
         dna_complement << "G"
        when "G"
         dna_complement << "C"
        when "T"
         dna_complement << "A"
        when "A"
         dna_complement << "U"
        end
      end
    dna_complement.join('')
  end
end

I am wondering what is the different between unless dna_structure.include? strand and if !(dna_strucutre.include?) in this case?

Answer 1

Since Ruby has not just if, but unless, you're encouraged to use it so long as the resulting code is clear. That is you should convert something like this:

if (!string.empty?)
  # ...
end

Into something like this:

unless (string.empty?)
  # ...
end

There's exceptions to this, like when you have this:

if (!string.empty?)
  # ... when not empty
else
  # ... when empty (when not not empty)
end

The naive approach would be to convert that to an unless but that creates a triple negative. You're already dealing with a double here, the else clause only happens if the string is not not empty, or arguably does not not not contain anything.

Do this instead:

if (string.empty?)
  # ... when empty
else
  # ... when not empty
end

There's a number of problems with the approach you're taking here, but the most grievous is that you're declaring a constant array inside your method each time the method is invoked. Since that never changes make it a constant at the top of the class level. At least:

class Complement
  DNA_STRUCTURE = %w[ C G A T ]
end

Even better would be to use a mapping table to represent the pairings:

COMPLEMENT = {
  'C' => 'G',
  'G' => 'C',
  'T' => 'A',
  'A' => 'U'
}.freeze

Now looking at your particular problem where you're trying to "invert" a given strand of characters, the tool you really want is tr on the string itself, a method that's optimized for handling things like cyphers where there's a 1:1 mapping between characters.

Your entire function collapses to this:

def self.of_dna(strand)
  strand.tr('CGTA', 'GCAU')
end

Now if you want to do a quick test to ensure you're actually dealing with a valid sequence:

def self.of_dna(strand)
  return '' unless (strand.match(/\A[CGTA]*\z/))

  strand.tr('CGTA', 'GCAU')
end

There's some other bad habits you're getting into here, like creating arrays to hold single characters when strings are much better at that particular task. c = '' and then c << 'G' would be more efficient than an array version of same, especially considering the array will hold N strings, each of which carries some overhead, and requires creating another string at the end using join. When using Ruby try and keep the number of objects required to do your computation, temporary or otherwise, to a minimum. It's usually faster with less "garbage".

Answer 2

Nothing wrong with the latter form but considering in ruby we have unless we should use it when we have just one branch and it's preferable, as in this case.

Anyway, it's exactly the same.