Your code is broken, and Ruby can help you fix it

Ruby is in many ways a better Perl, and it inherits a lot of its culture from the Perl community. One of the lessons I remember being hammered into my head early in the Perl community was the importance of putting Perl into verbose warnings mode:

Thou shalt use <strong>perl -w</strong> before moaning about Perl.

In fact, the Perl manpage goes on to list among Perl’s “bugs”:

The -w switch is not mandatory.

Perl was a very “loose” language, especially for its time; it would let you do stuff like refer to variables you hadn’t properly declared yet. -w was there to remind you that just because you can, doesn’t always mean you should. Putting Perl in warnings mode could save you from a multitude of careless mistakes, like misspelling a variable.

Ruby has a similar warnings mode, but sadly the practice of enabling verbose warnings by default has fallen by the wayside. This is unfortunate. Not only does it lead to avoidable bugs, it also forces folks like me who do make some effort to write warning-clean code to turn off verbose warnings because of the flood of warnings pouring out of common Rubygems.

Mislav wrote a post yesterday about Ruby’s warning system. I found parts of it helpful and informative, particularly the beginning, which contains a useful breakdown of Ruby’s assorted debug and verbosity flags and global variables. I think many of the issues cited, however, are better viewed as Ruby helpfully pointing out questionable coding practices—just like good old -w in Perl.

Lets go through them in order:

Undefined instance variable

The problem with instance variables that aren’t required to be explicitly declared and initialized is that it’s very easy to misspell them. Consider the following:

@recieved_message # => nil

Is that variable nil because no message was received? Or because the programmer misspelled “received” and is accidentally referencing the wrong variable? Verbose warnings mode would tell you:

@recieved_message # => nil # !> instance variable @message_recieved not initialized

As Mislav points out, modules complicate instance variable initialization. But perhaps not as much as he thinks. A good general rule for writing modules is to encapsulate every module-specific instance variable in its own idempotent auto-initializing accessor:

Module RoleSystem
  def role
    @role ||= :no_role_set
  end

  def set_role(role)
    @role = role.to_s
  end

  def is_role?(role)
    self.role == role.to_s
  end
end

class Person
  include RoleSystem
end

Person.new.is_role?('admin') # => false

Here, the job of making sure that @role is initialized is encapsulated in the #role method—no need for redundant checking in every method that references it. Ruby is lenient with regard to the ||= defaulting operator: it doesn’t print a warning when the variable being defaulted is undefined.

If that’s still too much code for your tastes, you can use a souped-up attributes library such as Ara T. Howard’s “fattr” to make it even more concise:

require 'fattr'

module RoleSystem
  fattr(:role) { nil } # attribute accessor for @role, defaulting to nil

  def set_role(role)
    @role = role.to_s
  end

  def is_role?(role)
    self.role == role.to_s
  end
end

class Person
  include RoleSystem
end

Person.new.is_role?('admin') # => false

As a side note, I consider having module-specific state to be an indicator that decoration/delegation may be called for rather than a mixin module; but that’s a post for another day.

Method redefined warning

In verbose warnings mode, Ruby warns you when you redefine a method. Considering the consternation that can ensue when methods are unexpectedly redefined, this is probably a Good Thing.

As it turns out, there is almost never a good reason to override methods in Ruby. Even in Rails, where it was once common practice, its use was stamped out once the maintainers realized that there were more robust techniques which achieved the same ends without any need for method redefinition.

About the only common reason to redefine methods is for short-lived kludges to get around some yet-to-be-patched third-party library defect. Arguably, such kludges should emit warnings, if only to encourage the developers to find a better solution post-haste.

However, as Mislav notes, it is occasionally desirable to redefine a method in certain metaprogramming scenarios. He gives the following example of the lengths you have to go to for a warning-free method redefinition that works in both 1.9 and 1.8:

undef :name if instance_methods.map {|m| m.to_sym }.include? :name
def name
  # ...
end

But there is a less ugly form that is equally portable and warning-free:

class Person
  attr_accessor :name

  undef :name if method_defined?(:name)

  def name
    @name.to_s.capitalize
  end
end

p = Person.new
p.name = "avdi"
p.name                          # => "Avdi"

As I said, however, this is rarely needed. It’s a lot cleaner to simply inject a module where you need to override methods:

class Person
  attr_accessor :name
end

module CapitalizedName
  def name
    super.to_s.capitalize
  end
end

p = Person.new
p.extend(CapitalizedName)
p.name = "avdi"
p.name                          # => "Avdi"

As you can see, this has the added benefit of giving easy access to the original method via super—no aliasing necessary.

Too verbose for you? Try this variation on for size:

class Person
  attr_accessor :name
end

p = Person.new
p.extend(Module.new do
  def name
    super.to_s.capitalize
  end
end)
p.name = "avdi"
p.name                          # => "Avdi"

“Useless use of == in void context”

This one crops up a lot in RSpec examples. There’s a simple fix, but it’s surprisingly little-known:

describe "equality" do
  let(:obj)   { 42 }
  let(:other) { 24 }
  specify {
    obj.should_not be == other
    obj.should be == obj
  }
end

Note the addition of “be” to the equality assertions. Just like that, no more warnings. As an added perk, this version reads better, especially for operators other than “==”:

    value.should > 23 # "value should greater than 23"
    value.should be > 23 # "value should BE greater than 23"

“Interpreted as argument prefix”

This refers to the case where Ruby notifies you that in code such as the following:

process *orders

The * operator will be interpreted as a “splat” rather than a multiplcation operator. Considering that the addition of a single space would completely change the meaning of the statement:

process * orders # process.*(orders)

I’m going to have to just plain disagree with Mislav on this one: that’s a good warning to have.

EDIT: José Valim objects that there are lots of places in Ruby code where inserting a space would break code, so why make a special case for & and *? The answer, I’d hazard to guess, is that in just about any other C-like language, whitespace around those operators is irrelevant. Ruby is making a special case for the operators most likely to be accidentally misused by programmers coming from other languages. That is, it’s compensating for a case where Ruby arguably does not adhere to the Principle of Least Surprise.

Lint versus verbose

Mislav sums up by saying that the real issue is that Ruby confuses its “lint” mode with “verbose” mode, and that it really should have two different modes: one where Ruby prints verbose programmer-inserted warnings, and another where it checks for common code issues. The thing is, Ruby does have these exact two modes; it’s just that by default it is already in the first mode. Consider the following code:

puts "Here we go..."
@not_defined
warn "This is a warning"

Let’s execute it with default interpreter options:

➜  ruby debug.rb
Here we go...
This is a warning

Now in “quiet” mode:

➜  ruby -W0 debug.rb
Here we go...

And now in “lint” mode:

➜  ruby -w debug.rb
debug.rb:2: warning: useless use of a variable in void context
Here we go...
This is a warning

As far as I can understand him, this is exactly the breakdown Mislav wants.

Me, I still tend to agree with the Perl manual: the fact that “-w” isn’t on by default is a bug. Here’s a challenge for you: start running your code under “-w”. You might just turn up a few latent bugs!

19 comments

  1. The undef :name if defined?(name) trick isn’t doing what you think it is.  defined?(name) returns true not because there is a name instance method but because there is a module method name (i.e. Person.name returns “Person”).  If you replace all the instances of name with first_name in that code, you’ll see that you still get the warning.

    Instead, I think you want undef :name if method_defined?(:name)–which does actually check to see if you have a name instance method.

    Do you know why “Useless use of == in a void context” goes away when you add the be?  I don’t understand what the be changes that would remove the warning.

    1. You used to get the result of the should method, then compare it using the == operator, then throw away the result. Since you haven’t used the result, you get the warning.

      With the be you instead get the result of the be method and compare it using the == operator, then, instead of throwing away the result, you pass it to the should method. Since you have used the result, you don’t get the warning.

  2. Sounds good. I’ve always been a big believer in automated error detection (as long as I’m given the facility to turn it off where I disagree). But how DO I run my Ruby code with warnings turned on, considering that it’s almost always via the rails, rspec or cucumber executables?

      1. Cool – thanks.

        Unfortunately, doing so reveals so many warnings in gems I use that -w is effectively useless. I suppose I could write a filter. Tell me, do people actually use -w in the real world?

        1. Yes, that is the sad state of affairs, where it’s nearly impossible to use
          it in production code because Ruby developers have ignored it for so long. I
          still try to write my gems to run clean.

          1. The other thing is that -w seems too heavy-handed. In the C++/C# worlds, I’m used to being able to turn warnings off – either globally, or just for a single section of code. For example, I do not want to be warned about uninitialized instance variables. That’s one of Ruby’s features that I like, and if there’s a problem with a misspelling, that just means there’s a missing test.

            It sounds like -w is something you should use every now and then (like rcov), to analyze possible problems but not to take as gospel.

  3. Very helpful.  Once upon a time I wrote a lot of Perl code and perl -w was indeed the only way I could keep my sanity.  Why I didn’t just bring that experience over to Ruby is beyond me and after reading this its like a light switch just flicked on.

  4. Ruby also doesn’t give a warning if you have attr_reader :role and then use self.role. Not that I’d recommend it, but still … (PS: Module in the third snippet needs to be downcased)

  5. Regarding method definition, there’s no need to undef a method here; if you don’t want the auto-generated reader, you can just not use attr_accesssor. In this case, attr_writer would do the job.

    1. In both my and Mislav’s examples, the attr_accessor is there for the sake of a simple example. A real method redefinition case would typically involve messing with some object whose definition you don’t control.

Comments are closed.