Helpers are for small snippets of code

Manfred Stienstra, 04 Jul 2007, 15:49 in ruby on rails and practices (edit).

Recently we contracted a local company to help out with a Rails project. Their primary job was to implement payments. During an audit or their work I found the following helper:

module CoursesHelper
  include ActiveMerchant::Billing 
  def issuers
    @gateway = IdealGateway.new(
         :merchant            => IDEAL_MERCHANT_ACCOUNT,
         :sub_id              => IDEAL_SUB_ID,
         :password            => IDEAL_PRIVATE_KEY_PASS,
         :private_cert        => IDEAL_PRIVATE_CERT,
         :language            => IDEAL_LANGUAGE,
         :private_key         => IDEAL_PRIVATE_KEY,
         :authentication_type => IDEAL_AUTHENTICATION_TYPE
    )
    
    list = Array.new()
    begin
      response = @gateway.issuers
      if response.success?
        list=Array.new()
        list.push({"issuerName"=>"Kies uw bank...", "issuerID"=>0})
        list=list + (response.params["list"])
      end
    rescue
      list = [{:issuerName=>"Kies uw bank...", :issuerID=>""},
      {:issuerName=>"ofline Simulator", :issuerID=>"00"}]
    end
    return list.map {|i| [ i["issuerName"], i["issuerID"] ] }

  end
  
end

Helpers are for keeping simple code out of your view. They should never include complicated logic. A helper should be so simple that you’d almost dare to deploy without testing.

Also, because it’s a good idea to keep coupling between classes to a minimum, the payment gateway should only be called from classes directly related to payment processing.

Finally, you don’t want such a large number of constants in your code. There are better ways to store the gateway configuration.

The contracter was asked to solve these issues. The gateway initialization was refactored to only appear once and the gateway object was assigned to a constant named IDEAL_GATEWAY. The code to find all issuers was moved to the Payment model.

class Payment < ActiveRecord::Base
  def self.ideal_issuers
    response = IDEAL_GATEWAY.issuers
    response.params["list"]
  end
end
module CoursesHelper
  
  def issuers
    list=Array.new()
    # 'Kies uw bank' means 'Choose your bank'
    list.push({"issuerName"=>"Kies uw bank...", "issuerID"=>0}) 
    list=list + Payment.ideal_issuers
    return list.map {|i| [i["issuerName"], i["issuerID"]] }
  end
  
end

Unfortunately, the issuers helper still looks more like Python than Ruby. I would have written it like this:

module CoursesHelper
  def issuers
    [['Kies uw bank...', 0]] + Payment.ideal_issuers.map \
      { |i|[i['issuerName'], i['issuerID']] }
  end  
end

I don’t think you should include instructions on how to use a drop-down as one of the options inside it. Furthermore, a default value makes no sense in this case. People always have to choose a bank, otherwise they can’t continue with the payment process.

After some refactoring I was left with the code below. I’ve also moved it to the helpers module for the payments controller, because that’s where the form is rendered.

module PaymentsHelper
  def issuers
    Payment.ideal_issuers.map { |i| [i['issuerName'], i['issuerID']] }
  end
end

8 comments

Adaptive planning

Thijs van der Vossen, 02 Nov 2006, 20:19 in practices (edit).

I very much like Martin Fowler’s latest Bliki entry on FeatureDevotion:

The key to beating off the waterfall is to realize that, as Dan puts it, agilists value Outcomes over Features. The feature list is a valuable tool, but it’s a means not an end. What really matters is the overall outcome, which I think of as value to the customers.

An important part of this thinking is that you expect the feature list to change as the project goes on. This happens you discover new things that you can do, and re-prioritize old things. This is the essence of adaptive planning, which has always been a key indicator of agile thinking. This results a big shift in how people think about a plan. In plan-driven projects, success and failure is often worded in terms of “did things go according to the plan?” In agile projects this is a meaningless question, because plans change so often. The plan is a tool, primarily one that you use to gauge the effect of changes: “how will adding this feature affect what we do”. The plan is a tool to figure out what should fit in the FivePoundBag. If your plan’s not constantly changing, you are very unlikely to be doing adaptive planning, and hence aren’t agile.

No comments yet

Getting Real, the book now free

Thijs van der Vossen, 26 Oct 2006, 09:11 in web, practices, and design (edit).

Getting Real, the book, self-published in PDF format this march by 37signals, is now also available as a paperback and in a free html version.

I never finished reading the pdf version because I find reading PDF documents on a computer screen really annoying.

Reading a well-designed html document from screen is a much, much better user experience; it’s easy to change font size, the position and width of page elements changes to fit the browser window and you can just scroll down and keep on reading without being interrupted by page boundaries.

For reading from screen, the html version of Getting Real just works better.

One thing is driving my crazy though; why on earth haven’t they used curved quotation marks in the html version? Proper punctuation just doesn’t matter anymore?

2 comments

Screencast Scripting

Manfred Stienstra, 09 Oct 2006, 22:07 in ruby on rails, practices, and video (edit).

Last week I posted a short screencast to show some features of ActiveSupport::Multibyte. After typing through the entire screencast twice I decided to automate the process. Screenager, the automated screencast typer, was born.

Download screencast (QuickTime, 544KB)

You can download Screenager from my personal Subversion repository, you will also need a recent version of ActiveSupport.

svn export https://dwerg.net/svn/screenager/trunk screenager
cd screenager
svn export --force http://svn.rubyonrails.org/rails/trunk/activesupport/lib
./screenager --speed 2 http://www.fngtps.com/files/2/2006/10/activesupport.rb

The version currently in SVN evaluates the Ruby code with eval using a clean binding every time you start a new screenplay. I really wanted to use the Freaky Freaky Sandbox for this, but it’s in heavy development and didn’t run at all when I was coding this. Sandbox and multiline Ruby statements are planned for future versions.

16 comments

It sounded so good on the golf course

Thijs van der Vossen, 16 May 2006, 13:24 in ruby on rails and practices (edit).

Martin Fowler in ’Evaluating Ruby’ (emphasis added):

Too many things are hard to judge that way - hence we spend so much of our time on client projects being slowed down by technology that sounded good on a golf course. My preference is to make this judgment based on experience - find people who have a track record for delivering in the mainstream environments and who have tried using Ruby.

No comments yet

Pair programming

Thijs van der Vossen, 10 Mar 2006, 14:41 in practices (edit).

Martin has taken some baby steps toward pair programming. One thing holding them back is that they work on al lot of different projects, so it’s rare for two people to be assigned to the same thing.

Although we’ve taken a lot of our practices from Extreme Programming, we don’t do full-time pair programming for much of the same reasons. I don’t see how pairing is going to help me when I’m designing a user interface, or when I’m doing server maintenance.

Having said that, we find ourselves programming in pairs more and more often. It really is much faster to solve complex problems together. It also works great for refactoring, because it’s much easier to keep track of all the changes when you’re working as a pair.

The only problem with pair programming is that it’s hard to explain to people who have no experience with it. Why would you want to have two programmers working on something that one can do alone? Isn’t that just wasting valuable programming hours?

Although there is a lot of evidence suggesting that it’s not, we’ve sidestepped the issue somewhat by only pairing for short periods of time when it’s really clear that it helps the project going forward.

No comments yet