Beautiful Code: The evolution of an iterator

Beautiful Code: The evolution of an iterator Jamis Nov 16

19 comments Latest by Seth Thomas Rasmussen

David, Marcel, and I had an interesting design-related discussion yesterday. But it wasn’t related to designing graphical UI’s — it was abut designing developer UI’s in code.

I was sharing some code I’d written related to the new data export feature we’re adding to Basecamp. The Export model supports a set of distinct states, “pending”, “processing”, and “completed”. I found myself iterating over those states in a few different places so I added a custom iterator to the model. This allowed me to centralize the work needed to do that loop:

  class Export < ActiveRecord::Base
    PENDING    = "pending" 
    PROCESSING = "processing" 
    COMPLETED  = "completed" 

    STATES     = [PENDING, PROCESSING, COMPLETED]

    def self.each_state
      STATES.each { |state| yield state }
    end

    # ...
  end

The custom iterator is then used something like this:

  class ExportPresenter
    # ...

    Export.each_state do |state|
      class_eval "def #{state}?; @export && @export.#{state}?; end" 
    end

    # ...
  end

Some discussion ensued:

David H.	You find that more readable than just Export::STATES.each ?
Jamis B.	yah
Jamis B.	I really find constants, in general, rather ugly
David H.	Hmm.. I actually regret doing my own iterator for errors in AR
David H.	From the smalltalk book, Kent advices that you turn constants into methods
David H.	so you’d have
David H.	Export.states
David H.	the problem I have with custom iterators is that there are too many
David H.	and often times you don’t want each, but you want collect
David H.	or select
David H.	or each_with_index
Jamis B.	actually, I like that. Export.states.each

The result was much cleaner, and allowed for the full gamut of Array operations to be performed on Export.states.

  class Export < ActiveRecord::Base
    PENDING    = "pending" 
    PROCESSING = "processing" 
    COMPLETED  = "completed" 

    def self.states
      @states ||= [PENDING, PROCESSING, COMPLETED]
    end

    # ...
  end

  class ExportPresenter
    # ...

    Export.states.each do |state|
      class_eval "def #{state}?; @export && @export.#{state}?; end" 
    end

    # ...
  end

Jamis B.	I like that better
David H.	exactly
Marcel M.	that’s a cool little pattern
Mark I.	That reads much better than Export::STATES does too.
Jamis B.	yah
Marcel M.	WHY DO YOU ALL HATE EIFFEL SO MUCH

Looking for a job? Got a position to fill? Check out the Job Board.

Got a web design project in mind? Find a web designer on Haystack. Browse by visual style, portfolio, budget, and geographic location.

Over 1 million people use 37signals' simple web-based software to collaborate on projects, track contacts, and organize their business with an intranet.

19 comments so far

Lucas Carlson 16 Nov 06

Why use constants like PENDING = “pending” in a language that has symbols?

Jamis 16 Nov 06

Because databases don’t have symbols, and the strings represent the discrete set of values that the ‘state’ column can contain.

qwerty 16 Nov 06

Export.states is now writable where it wasn’t in the Iterator solution (not sure – I’m not a fluent in Ruby). Thus, any client may modify it. To prevent this, you could also return a copy of that array and then let the client use each() or whatever they please.

Actually, in Ruby, constants are not immutable. Ruby just gets annoyed if you try to reassign a constant, but you can alter the object that a constant points to as much as you want.

However, Ruby objects do sport a “freeze” method, which would do what you expect (make the object immutable). However, given that this is internal code, we don’t have to be quite so paranoid. :)

ste 16 Nov 06

qwerty, actually Export.states is “safer” than Export::STATES, and here’s why:

irb(main):005:0> Export.states = 1 NoMethodError: undefined method `states=’ for Export:Class from (irb):5 from :0 irb(main):006:0> Export::STATES = :foo (irb):6: warning: already initialized constant STATES => :foo

of course you can always do

Export.states 0 =:rubbish

but the same applies to Export::STATES.

@ste: thanks for the explanation. I was indeed referring to Export.states.push(“foo”) and the like which modifies the array.

There are still some subtleties that I don’t understand: self.states defines a class-method Export.states (rather than an object method). Therefore states should be a class attribute but was expecting this to be named @states. I guess I miss something here.

erik 16 Nov 06

I don’t get it- why not just use :attr_reader?

Note that attr_reader is for instances of a class, not for the class itself. Also, even if you use attr_reader, you still have to initialize it somehow.

You’ll find that @foo ||= “something” is a pretty common idiom for memoizing methods in Ruby. In this case, I have a class-level attribute that I want to be able to query (the array of possible export states). I could just return the array each time, but that’s inefficient, since Ruby will create a new array every time it is called. Thus, I store the result once in an instance variable, and then just return the value of that variable in subsequent calls.

Note that classes, in Ruby, are Object instances, too. (Everything, or just about everything, in Ruby is an Object). Thus, classes can have instance variables, too, which are independent of the variables in the instances of that class… kind of crazy, but if you just accept it as given, life gets a lot nicer. :) Blind faith!

I’d strongly recommend “Programming Ruby”, by the Pragmatic Programmers, if you’d like to delve further into this. Or, if you tend to prefer the bizarre, there is also why’s Poignant Guide to Ruby.

Damien Pollet 16 Nov 06

What’s the final joke with Eiffel ? @states ||= [... does look like a “once” feature, but I don’t get the link with iteration methods…

The ALL CAPS thing. Eiffel gets caps happy. :)

qwerty 17 Nov 06

Classes in Java,C++, Python, Smalltalk and probably many more languages can have attributes, too, so it is a common OO feature. In Java these are declared “static”. This is also independent from whether classes are objects (like in Smalltalk) or not (like in Java).

Back to my initial comment: by returning a reference to a mutable array you are revealing your internal implementation. Returning a copy would be cleaner, as would be the iterator.

Peter Cooper 17 Nov 06

You’re not storing them as plain text though, right? For enumerations, a TINYINT is sufficient and only takes a single byte, rather than the possible 11 for ‘processing’ in a VARCHAR (ENUM is nicer, of course, but more resistant to change once your table is a gazillion rows long).

Rodrigo K 17 Nov 06

You could check the inclusion within a String array that stores in the db the string equivalent of the symbol.

validates_inclusion_of :state, :in => ["pending","processing","completed"] def state read_attribute(:state).to_sym end def state=(value) write_attribute(:state, value.to_s) end

Just an idea :)

Jamis 17 Nov 06

Very true, Peter and Rodrigo, either of those approaches would work. But both require more code for something that doesn’t need more code. There won’t be millions of export records in the database, and it’s only a tiny niche corner of the app, so it’s just not worth adding more code to manage it. More code == more maintenance. We’re going “path of least resistance” here. :)

Bryan Helmkamp 17 Nov 06

Jamis,

Even though it is besides the point of this post, could you explain a little bit about how you are using Presenter models in your apps? It’s not something I’ve seen discussed much anywhere.

Or perhaps that might be a good topics for a post on The Rails Way.

Thanks.

Bryan, I do intend to talk about it on The Rails Way eventually, but for you now, you can read the post that originally inspired David and I, Rails Model View Controller + Presenter? by Jay Fields.

topfunky 21 Nov 06

It may not be needed in this situation, but Scott Barron’s acts_as_state_machine is awesome for recording state and firing of events when they change. It has a nice Ruby syntax, too.

http://rubyi.st/2006/1/21/acts-as-state-machine

Seth Thomas Rasmussen 22 Nov 06

I’ve grown pretty fond of camel case constants beyond that which are for modules, for the benefit of the eyes. I’ve thought about using methods all the time, but I think there is some value to the semantics of a constant that are worth keeping. I sort of generally think of anything else as open for meddling. ;)

That said, I also like methods which refer to a constant using the scope operator instead of a dot, to clarify said semantics. These need not return bona fide constants, mayhaps.

This is Signal vs. Noise, a weblog by 37signals about design, business, experience, simplicity, the web, culture, and more. Established 1999 in Chicago. Visit the Product Blog for more information on our products.

Jobs: