I've had some people tell me that they didn't know about the splat operator in Ruby (*array) so here's a quick example of how it's used.

> array = [:a, :b, :c]
[
    [0] :a,
    [1] :b,
    [2] :c
]
>> a, b, c = *array
[
    [0] :a,
    [1] :b,
    [2] :c
]
>> a
:a
>> b
:b
>> c
:c

It splits the elements of the array into single items which are returned as a group (I think they are called parameters). So in second statement in the example above:

  • a gets the first element of array, :a
  • b gets the second element of array, :b
  • c gets the third element of array, :c

This is commonly used to set a bunch of variables or when working with a method's arguments (e.g. in a monkey patch).

Tagged: ruby

Today I'm looking at facets' Hash#to_struct. I use structs in my code when I want to store some complex data but don't need to full Ruby class.

The Code

# File lib/core/facets/hash/to_struct.rb, line 12
  def to_struct(struct_name)
    Struct.new(struct_name,*keys).new(*values)
  end

Example

#!/usr/bin/ruby -wKU
#
# Code Reading #3
require '../base'
require 'facets/hash/to_struct'

Struct.new("Application", :users, :servers)

class Example
  def self.create
    {
      :users => [:edavis],
      :servers => [:app1, :app2, :app3]
    }.to_struct("Application")
  end
end

ap(Example.create)

class HashToStruct < Test::Unit::TestCase
  def test_should_be_a_struct
    assert_kind_of Struct, Example.create
  end

  def test_should_set_users
    @example = Example.create
    assert_equal [:edavis], @example.users
  end
  
  def test_should_set_servers
    @example = Example.create
    assert_equal [:app1, :app2, :app3], @example.servers
  end
  
end

On github

Review

Hash#to_struct has a pretty simple implementation.

  1. First it defines a new Struct class called "Application" with the attributes of 'users' and 'servers'.
  2. This returns a new Class object, which #to_struct calls #new on passing in the Hash's values (think of it as Struct::Application.new(*values)).
  3. Then the attributes are set, and the Struct::Application object is returned.

So in reality, I should revise my first sentence "I use structs in my code when I want to store some complex data but I don't want to write full Ruby class myself."

Tagged: code-reading facets hash

I'm continuing my look at Ruby Facets today, this time looking at another Hash method. I use Hashes everyday in Rails programming so if I can pick up some new techniques, I can put them to use right away. Today's subject is Hash#zipnew.

The Code

# File lib/core/facets/hash/zipnew.rb, line 11
  def self.zipnew(keys,values) # or some better name
    h = {}
    keys.size.times{ |i| h[ keys[i] ] = values[i] }
    h
  end

Example

#!/usr/bin/ruby -wKU
#
# Code Reading #2
require '../base'
require "facets/hash/zipnew"

class HashZipnew
  def data
    keys = [
            '2010-05-15',
            '2010-05-16',
            '2010-05-17',
            '2010-05-18',
            '2010-05-19'
            ]
    values = [
              100.0,
              450.5,
              89.0,
              0.0,
              78.5
             ]
    Hash.zipnew(keys, values)
  end
end

ap(HashZipnew.new.data)

class HashZipnewTest < Test::Unit::TestCase
  def test_use_the_arrays_to_map_keys_to_values
    hashish = HashZipnew.new.data

    assert_equal 100.0, hashish['2010-05-15']
    assert_equal 450.5, hashish['2010-05-16']
    assert_equal  89.0, hashish['2010-05-17']
    assert_equal   0.0, hashish['2010-05-18']
    assert_equal  78.5, hashish['2010-05-19']
  end
  
end

On github

Review

Hash#zipnew is a awesome method. I can't remember how many times I've collected two sets of time based data and had to merge them together, like when graphing time data. Hash#zipnew is pretty simple to use, just feed it two arrays of data; one for the keys and one for the values.

Internally, Hash#zipnew takes the size of the keys array and adds a new key/value to a hash. Running the loop by hand in irb shows this for the first two elements:

>> h = {}
{}
>> h['2010-05-15'] = 100.0
100.0
>> h['2010-05-16'] = 450.5
450.5
>> ...

I really like how #zipnew makes use of Array#size and Fixnum#times to automatically control it's loops.

Since code reading should be an active activity, I've also created an alternative implementation that uses Array#with_index which would remove the #size and #times method calls.

  def self.alt_zipnew(keys, values)
    h = {}
    keys.each_with_index {|key, index| h[ key ] = values[ index ] }
    h
  end

This is the reason I'm interested in rubinius. Since they are using Ruby for (almost) the entire virtual machine, anyone can dig into the Ruby source code and play around with the internal implementation.

Tagged: code-reading facets hash

I'm starting this series by taking a look at Ruby Facets. Facets is a collection of Ruby core and Ruby standard library extensions. It has a lot of good code embedded in it from many contributors so it should be a good place to start looking for some new ideas.

The Code

# File lib/core/facets/hash/autonew.rb, line 19
  def self.autonew(*args)
    #new(*args){|a,k| a[k] = self.class::new(*args)}
    leet = lambda { |hsh, key| hsh[key] = new( &leet ) }
    new(*args,&leet)
  end

Example

#!/usr/bin/ruby -wKU
#
# Code Reading #1
require '../base'
require 'facets/hash/autonew'

class HashAutonew
  def data
    @data = Hash.autonew
    @data['users']['eric']['blog'] = 'http://theadmin.org'
    @data
  end
end

ap(HashAutonew.new.data)

class HashAutonewTest < Test::Unit::TestCase
  def test_should_nest_values_automatically
    hashish = HashAutonew.new.data

    assert hashish.key?('users')
    assert hashish['users'].key?('eric')
    assert hashish['users']['eric'].key?('blog')
    assert_equal 'http://theadmin.org', hashish['users']['eric']['blog']
  end
  
end

On github

Review

Hash#autonew is wrapping Hash#new using #new's block form. I can't dig to deep into Hash#new because it's implemented in C but it looks like the lambda is using recursion to set a bunch of default values for the hash when it's accessed with child elements. Hash#new supports this but it only works one level deep:

>> Hash.new
{}
>> Hash.new{|hash, key| 'default'}
{}
>> h = Hash.new{|hash, key| 'default'}
{}
>> h['s']
"default"
>> h['s']['s']['s']
NoMethodError: undefined method `[]' for nil:NilClass
        from (irb):5

So in my test:

  1. @data is getting a 'users' key with the value of
  2. a new hash with the key 'eric' with the value of
  3. a new hash with the value of 'http://theadmin.org'
  4. (recursion stops)

The recursion stops with the 'http://theadmin.org' string because #autonew is passing String#new the leet block but String#new doesn't do anything with the block and just returns the string.

Sidebar: Precise loading

I noticed that facets provides many different ways to load it's extensions:

  • require 'facets' - load everything
  • require 'facets/hash' - only load the Hash extensions
  • require 'facets/hash/autonew' - only load the Hash#autonew extension

This is very useful since it lets a developer only extend what they need and is a nice change from ActiveSupport which loads everything at once.

Tagged: code-reading facets hash

My daily refactorings have improved my existing code up to my current knowledge of Ruby. Now I need a way to learn new ideas and ways to write Ruby. I'm going to start doing a daily code reading and review every day:

  1. posting it
  2. reading over it
  3. taking notes on the interesting techniques
  4. try to write some simple code to make use of it

This repetition will help me remember the different techniques and styles of other developers, just like I described in 2 Steps To Becoming A Great Developer.

I'm going to start with some libraries I've been wanting to dig into:

If you have any other project suggestions, please post a comment below.

Tagged: code-reading

After almost four months of daily refactors, I've decided to pause them. They have been very successful and helped me overcome a lot of the fears I've had with programming. I just feel that they have lost some of their value now and I want to move on to another interesting idea for self training.

I'm still going to do daily refactors, I have too much code that I can't let rot. If you're interested in what I'm refactoring, follow my Github activity stream.

Tagged: refactoring

The Refactoring

Today I used move method on the InvoiceController to move the #last_invoice_number to the Invoice model

Before

class InvoiceController < ApplicationController
  def last_invoice_number
    last_invoice = Invoice.find(:first, :order => 'id DESC')
    unless last_invoice.nil?
      last_invoice.invoice_number
    else
      '-'
    end
  end
end
class Invoice < ActiveRecord::Base
  # ...
end

After

class InvoiceController < ApplicationController
  # ...
end
class Invoice < ActiveRecord::Base
  def self.last_invoice_number
    last_invoice = first(:order => 'id DESC')
    if last_invoice.present?
      last_invoice.invoice_number
    else
      '-'
    end

  end
end

Review

I'm really starting to believe that the only methods in a controller should be actions or filters. Everything else should me moved up to ApplicationController, down to the Model, or sideways into Helpers. Controllers still seem to be the default place code is added in Rails and the place that always has all the bad code.

Reference commit

Tagged: ruby redmine refactoring move-method

The Refactoring

Following up on yesterday's refactoring, today I converted the PaymentsController to a REST controller.

Before

# config/routes.rb
ActionController::Routing::Routes.draw do |map|
  map.resources(:invoice,
                :collection => {
                  :autocreate => :get,
                  :autofill => :get
                },
                :member => {
                  :outstanding => :get
                })
end
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"

class RoutingTest < ActionController::IntegrationTest
  context "payments" do
    should_route :get, "/payments/new", :controller => 'payments', :action => 'new'
    
    should_route :post, "/payments/create", :controller => 'payments', :action => 'create'
  end
end

After

# config/routes.rb
ActionController::Routing::Routes.draw do |map|
  map.resources(:invoice,
                :collection => {
                  :autocreate => :get,
                  :autofill => :get
                },
                :member => {
                  :outstanding => :get
                },
                :shallow => true) do |invoice|
    invoice.resources :payments, :only => [:new, :create]
  end

  map.resources :payments, :only => [:new, :create]
end
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"

class RoutingTest < ActionController::IntegrationTest
  context "payments" do
    should_route :get, "/payments/new", :controller => 'payments', :action => 'new'
    
    should_route :post, "/payments", :controller => 'payments', :action => 'create'
    
    should_route :get, "/invoice/100/payments/new", :controller => 'payments', :action => 'new', :invoice_id => '100'
    
    should_route :post, "/invoice/100/payments", :controller => 'payments', :action => 'create', :invoice_id => '100'
  end
end

Review

There is one smell I'd like to figure out from this refactoring. I couldn't find why I needed to define both a shallow route (invoice.resources :payments) and a regular route (map.resources :payments). If I only had the shallow one defined, I couldn't access PaymentsController#create at '/payments/ because it fell through to the default route (and the index action). If anyone has a suggestion about what's wrong, let me know in the comments below.

Reference commit

Tagged: ruby redmine refactoring

The Refactoring

Today's refactoring took me quite a bit of setup work. I wanted to convert the InvoiceController to a REST controller in order to take advantage of all the built in helpers in Rails. I'm only going to show the routing changes I made, if want to dig deeper into the change there are view and test changes in the commit.

Before

# config/routes.rb
# ... Empty ...
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"

class RoutingTest < ActionController::IntegrationTest
  context "invoices" do
    should_route :get, "/invoice/index/name", :controller => 'invoice', :action => 'index', :id => 'name'
    should_route :get, "/invoice/new/name", :controller => 'invoice', :action => 'new', :id => 'name'
    should_route :get, "/invoice/autocreate/name", :controller => 'invoice', :action => 'autocreate', :id => 'name'
    should_route :get, "/invoice/show/name", :controller => 'invoice', :action => 'show', :id => 'name'
    should_route :get, "/invoice/edit/name", :controller => 'invoice', :action => 'edit', :id => 'name'
    should_route :get, "/invoice/autofill/name", :controller => 'invoice', :action => 'autofill', :id => 'name'
    should_route :get, "/invoice/outstanding/name", :controller => 'invoice', :action => 'outstanding', :id => 'name'


    should_route :post, "/invoice/create/name", :controller => 'invoice', :action => 'create', :id => 'name'
    should_route :post, "/invoice/update/name", :controller => 'invoice', :action => 'update', :id => 'name'
    should_route :post, "/invoice/destroy/name", :controller => 'invoice', :action => 'destroy', :id => 'name'
  end

end

After

# config/routes.rb
ActionController::Routing::Routes.draw do |map|
  map.resources(:invoice,
                :collection => {
                  :autocreate => :get,
                  :autofill => :get
                },
                :member => {
                  :outstanding => :get
                })
end
# test/integration/routing_test.rb
require "#{File.dirname(__FILE__)}/../test_helper"

class RoutingTest < ActionController::IntegrationTest
  context "invoices" do
    should_route :get, "/invoice", :controller => 'invoice', :action => 'index'
    should_route :get, "/invoice/new", :controller => 'invoice', :action => 'new'
    should_route :get, "/invoice/autocreate", :controller => 'invoice', :action => 'autocreate'
    should_route :get, "/invoice/100", :controller => 'invoice', :action => 'show', :id => '100'
    should_route :get, "/invoice/100/edit", :controller => 'invoice', :action => 'edit', :id => '100'
    should_route :get, "/invoice/autofill", :controller => 'invoice', :action => 'autofill'
    should_route :get, "/invoice/100/outstanding", :controller => 'invoice', :action => 'outstanding', :id => '100'


    should_route :post, "/invoice", :controller => 'invoice', :action => 'create'

    should_route :put, "/invoice/100", :controller => 'invoice', :action => 'update', :id => '100'

    should_route :delete, "/invoice/100", :controller => 'invoice', :action => 'destroy', :id => '100'
  end

end

Review

This refactoring, combined with removing the project requirement, has really helped to update the design of the Invoice Plugin. After a few more bug fixes, it should be ready for it's first official release (over two years in the making).

Reference commit

Tagged: ruby redmine refactoring

The Refactoring

Today's refactoring cleans up some of the stubs left from last Friday.

Before

class InvoiceController < ApplicationController
  def autofill
    @autofill = Autofill.new_from_params(params[:autofill])

    # TODO: should just access @autofill directly
    @p = @autofill.p
    @customer = @autofill.customer
    @date_from = @autofill.date_from
    @date_to = @autofill.date_to
    @activities = @autofill.activities
    @issues = @autofill.issues
    @total_time = @autofill.total_time
    @time_entries = @autofill.time_entries
    @total = @autofill.total
    
    respond_to do |format|
      format.js
    end
  end
end
# app/views/invoices/autofill.js.rjs
page['invoice_amount'].value = @total
page['invoice_project_id'].value = @p.id

# Invoice description
description = ''
description << "h3. " + h(@p.name) + "\n\n"
description << h(@p.description) + "\n\n"
description << "h3. Work Completed\n\n"

@issues.each do |issue|
  description << "* #{issue.subject} (#{issue.status} ##{issue.id})\n"
end

# Total time
description << "\n"
description << "Total billable time: " + pluralize(@total_time, "hour")

description << " (" + h(@date_from) + " to " + h(@date_to) + ")"

page['invoice_description'].value = description

# Customer
if @customer.blank?
  page.alert l(:label_no_customer_on_project)
else
  page['invoice_customer_id'].value = @customer.id
  page.replace_html 'customer_name', h(@customer.name)
end

After

class InvoiceController < ApplicationController
  def autofill
    @autofill = Autofill.new_from_params(params[:autofill])
    
    respond_to do |format|
      format.js
    end
  end
end
# app/views/invoices/autofill.js.rjs
page['invoice_amount'].value = @autofill.total
page['invoice_project_id'].value = @autofill.p.id

# Invoice description
description = ''
description << "h3. " + h(@autofill.p.name) + "\n\n"
description << h(@autofill.p.description) + "\n\n"
description << "h3. Work Completed\n\n"

@autofill.issues.each do |issue|
  description << "* #{issue.subject} (#{issue.status} ##{issue.id})\n"
end

# Total time
description << "\n"
description << "Total billable time: " + pluralize(@autofill.total_time, "hour")

description << " (" + h(@autofill.date_from) + " to " + h(@autofill.date_to) + ")"

page['invoice_description'].value = description

# Customer
if @autofill.customer.blank?
  page.alert l(:label_no_customer_on_project)
else
  page['invoice_customer_id'].value = @autofill.customer.id
  page.replace_html 'customer_name', h(@autofill.customer.name)
end

Review

By changing the data access so it goes through the @autofill instance, I don't have to track as many instance variables in the controller and have the flexibility to change any of the variables' implementation later. This will let me refactor that RJS away into a cleaner interface later.

Reference commit

Tagged: ruby redmine refactoring replace-data-value-with-object