Working in Rails and dealing with Rails code and various bits of Ruby in plugins etc. it is easy to forget some OO fundamentals. This is partly due to the power of Ruby and partly due to specialist classes like controllers, models and specs distracting me away from the fact that all these things are still classes.

There used to be a rule of thumb used in Java that no method should be more than 10 lines long. Personally I took this down to 5 lines. Ruby is so powerful, that in 10 lines we can create something seriously complex, in fact we can do this in five lines. The problem with these rules of thumb is that they seem pretty stupid unless you know the underlying reasoning behind them. So lets explore this a bit further and remember how to write classes properly

Public, Private

The methods a class responds to are its public interface. This interface should document how the class works. The code that implements this interface has two purposes

1. To further document intention, i.e. to describe what we are doing
2. To do stuff

Surprise, surprise - the first is far more important than the second.

Learning from Java

When writing in clunkier languages like Java it becomes particularly obvious that public methods should actually delegate doing stuff to private methods. Well written java classes will have a small number of public methods, supported by a large number of private methods which actually do the work.

This is an old bit of java I wrote some time ago. The good thing about this code is that the public method clearly explains what is happening

public class Parser {
  
  ... // twenty lines of code defining state in private variables - including out
  
  public StringBuffer parse(File in) throws FileNotFoundException, IOException, ParserException {
    this.in = in;
    checkParseArgs();
    checkFileExists();
    openFile();
    initialiseStorage();
    visitedFiles.add(in);
    setCurrentWorkingDirectory();         
    while (getNextInclude()){
      processInclude();
    }
    return out;
  }
  
  ... // 230 lines of code and comments defining 21 void private no arg methods that actually do the work
}

Of course we don’t want to go back to the verbosity of java, but in becoming much more line efficient in our code, we shouldn’t forget the importance of clarity of public methods. If I turn this into English I get

Parser parses a file by,

• checking the parse args
• checking the file exists
• opening the file
• initialising the storage
• adding the file to the visited file list
• setting the current working directory
• finally getting each include and processing it

Now this isn’t perfect - I don’t know what an include is (yet), but its fairly close to self documenting code, and its easy to find out things by just navigating to the private methods

The private methods in this class either look like the public method, or do one thing. I’ll show a few of them below

private void checkParseArgs(){
    if (in == null) {
        throw new IllegalArgumentException("Parser.parse() will not except null arguments");
    }
}
  
  
private void processInclude() throws IOException, ParserException{       
    getIncludeFile();
    parseIncludeFile();
    replaceIncludeWithFile();
}  

private void getIncludeFile() throws IOException, ParserException{
    fReader.mark(tagStartInclude.length() + 255);
    readToStartOfPath();
    readPath();
    readEndTag();
}

private boolean isComment() throws IOException {
    int len = tagStartComment.length();
    char [] buf = new char[len];
    fReader.mark(len);
    fReader.read(buf,0,len);
    String s = new String(buf);
    fReader.reset(); // back to start of tag
    return tagStartComment.equals(s);
}

OK so in Ruby terms this is all very dull and long winded and we can do things much quicker and terser. But what I’d like to keep from this code is the idea that even in Ruby public class methods should clearly document their intention and in general delegate to private methods to do stuff. I’d also like to keep the idea that a method either does something or tells you how something is done - but not both. Finally I can remove all complex conditionals from my ruby code using these techniques. No need for case statements and definitely no need for nested ifs!

By the way, I wrote this parser class 7 years ago. Coming back to it, it took me about 5 minutes to work out what it does and how it does it, and this reminded me of why I wrote it. It would be nice if the ruby code I write today would be so clear in 7 years time!

Ruby Classes

With Ruby its very easy to forget about private methods and just implement 10 line public methods. From one of my Rails models I have

def weight_in_kg=(weight)
  if weight.nil? || weight == ""
    self.weight_in_grammes = weight if self.weight_in_grammes.nil?
  else
    strip = weight.strip[@@strip_regex]
    split = strip.split('.',2) unless strip.nil?
    if split.nil? || strip.nil?
      self.weight_in_grammes = weight if self.weight_in_grammes.nil?
    else
      kg = (split.first).to_i
      gms = split.size > 1 ? (split.last).to_i : 0 
      self.weight_in_grammes = kg * 1000 + gms
    end
  end
end

Now this code is just so much harder to read than the java code above. At 12 lines its not long and in Ruby its just so easy to write! And with my unit tests I can get away with this code. But I wrote this about 9 months ago - and I have less idea of what it is doing than the java code I wrote 7 years ago! Create a few hundred methods like this and you’ll soon have no idea of what your code is doing.