yes blocks and metaprogramming are very clever...

…but if you can’t read the code without bashing your head against a wall, its pretty useless.

Old blog posts I never published, except I have now #2

Now I admit that this solution has some merit, it filters incoming params in a particular controller action and DRYs up some of the saving and redirecting that is common place in every controller. But when I took this project on it just complicated things so much that it took 4 times as long to make any changes to the app as it needed to.

do_object_edit("recruiter/new_subscription", :pay_subscription, :agreed_terms, :subscription_type_id) do |o|
  o.transaction_detail = "INCOMPLETE"
  o.amount_paid = 0
end

def do_object_edit(template, action, *fields, &block)
  logger.debug "in do_object_edit"
  logger.debug "allowing edit of #{fields.join(', ')}" unless fields.empty?
  if submitted_using_button?("Cancel")
    if action.is_a?(Symbol)
      redirect_to :action => action
    else
      redirect_to action
    end
    return
  elsif request.post?
    fields.map!{|f| f.to_sym}
    params[:object].delete_if{|k,v| !fields.include?(k.to_sym)} unless fields.empty?
    @object.attributes = params[:object]
    begin
      ActiveRecord::Base.transaction do
        if block_given?
          yield @object
        end
        @object.save!
        flash_message "Details Saved"
        if action.is_a?(Symbol)
          redirect_to :action => action
        else
          redirect_to action
        end
        return
      end
    rescue ActiveRecord::RecordInvalid
      logger.warn $!
      flash_message "An Error Occurred"
    end
  end
  render :template => template
end

Yes is DRY because this is repeated everywhere in some form or other, but its not as readable as this.

s = Subscription.new(params[:subscription]) do |s|
  s.transaction_detail = 'INCOMPLETE'
  s.amount_paid = 0
end

unless s.save
  redirect_to failure
else
  redirect_to success
end

So as another developer taking on a dead project, this just made things harder. So my comment really is, coding an application isn’t just for you its for the client and if you can’t read it easily a month after you’ve written it someone else won’t be able to either.

It stinks of coding arrogance over creating a maintainable project. And the client loses…because it takes 5 times longer to get someone to fix it or make changes in the future.