Nick

[Ruby] On DRY up a controller action

2011-08-05T12:42:35-07:00

@tazsingh I prefer to keep the #render calls as they are, because I'm using the JSend standard:
http://labs.omniti.com/labs/jsend

Here's what I've got now.

respond_to do |format|
  if @image.update_attributes params[:product_image]
    format.js   { render :json => { :status => 'success', :data => {:message => I18n.t('product_image.actions.updated')} } } 
    format.html { redirect_to main_curate_product_images_path(@product), :notice => t('product_image.actions.updated') }
  else
    format.js   { render :json => { :status => 'fail', :data => {:message => I18n.t('product_image.actions.update_failed')} } } 
    format.html do
      @catalog          = @product.catalog
      @parent_catalogs  = @catalog.ancestors.only :slug, :name
      @crop_data        = @image.crop_data
 
      render :action => :edit
    end 
  end
end

[Ruby] On DRY up a controller action

2011-08-05T11:51:31-07:00

@tazsingh suggested using #respond_to .

if @image.update_attributes params[:product_image]
  respond_to do |format|
    format.json { render :json => { :status => 'success', :data => {:message => I18n.t('product_image.actions.updated')} } }
    format.html { redirect_to main_curate_product_images_path(@product), :notice => t('product_image.actions.updated') }
  end
else
  respond_to do |format|
    format.json { render :json => { :status => 'fail', :data => {:message => I18n.t('product_image.actions.update_failed')} } }
    format.html do
      @catalog          = @product.catalog
      @parent_catalogs  = @catalog.ancestors.only :slug, :name
      @crop_data        = @image.crop_data

      render :action => :edit
    end
  end
end

[Ruby] DRY up a controller action

2011-08-05T11:13:10-07:00

Any suggestions for DRYing up this portion of a controller action?

if @image.update_attributes params[:product_image]
  if request.xhr?
    render :json => { :status => 'success', :data => {:message => I18n.t('product_image.results.reordered')} }
  else
    redirect_to curate_product_images_path(@product), :notice => t('product_image.actions.updated')
  end 
else
  if request.xhr?
    render :json => { :status => 'fail', :data => {:message => I18n.t('product_image.results.reorder_failed')} }
  else
    render :action => :edit
  end 
end

[Ruby] On Condensing nested conditionals

2008-11-12T17:56:53-08:00

Interesting solution, Adam! Much cleaner than my original code. Thanks, mate. Much appreciated!

[Ruby] On Condensing nested conditionals

2008-11-12T00:40:28-08:00

Hi Adam. You're correct about #deny_access; it uses #redirect_to to halt the filter chain. Your refactoring is great, though it dropped one bit of functionality that's in the original post: redirecting the browser to :back if the requested action doesn't require authentication and hasn't been made public. Any thoughts on how to incorporate that into your suggestion?

Thanks,
Nick

[Ruby] Condensing nested conditionals

2008-11-11T21:28:53-08:00

The code below works, but feels a bit overly complex, and looks ugly. Any suggestions on how to condense it?

## app/controllers/properties_controller.rb
class PropertiesController < ApplicationController
  before_filter :authorise_action

  private

    def authorise_action
      case action_name
      when 'index', 'show', 'map_info_window'
        # Allowed by anyone.
      when 'new', 'create', 'edit', 'update', 'destroy'
        if is_logged_out?
          deny_access :notice => Phrases.login_required, :redirect_to => login_url
        elsif is_landlord? or is_admin?
          # Allowed.
        else
          deny_access :notice => Phrases.access_denied, :store_location => false
        end
      else
        deny_access :notice => Phrases.not_public_yet, :store_location => false, :redirect_to => :back
      end
    end
end

[Ruby] On DRY up a controller action

2008-11-03T05:46:12-08:00

Hey Adam. There's a Property model, which originally containted what's now the PropertyFilter model. Basically, I needed to do large, multi-param finds, so started building it into the Property model. After a few iterations and feedback from others, I moved the processing of the search params, the building of the args to #find , and the call to #find out of the Property model and into a new model, PropertyFilter.

Because PropertyFilter doesn't require data to persist, it inherits from AR::BaseWithoutTable .

[Ruby] On DRY up a controller action

2008-11-01T17:10:19-07:00

Thanks Jarkko. The action's a lot more concise without those instance variable definitions up top, and the calls to #render_to_string .

I have a habit of defining variables at the beginning of my methods...Perl's "use strict;" battered that into my head many years ago.

[Bash] On Groupping with stats

2008-11-01T16:57:58-07:00

I'd write a small script for this. I've been using Ruby lately, so here's one solution:

## process_stats.rb [ruby]
filename    = 'process.log'
ids         = {}

File.readlines(filename).each do |line|
  id = line.match(/\d+/)[0]
  ids[id] ? ids[id] += 1 : ids[id] = 1
end

ids.sort {|a,b| a[1] <=> b[1]}.reverse.each do |pair|
  puts "#{pair[1]} :#{pair[0]}"
end

[Ruby] DRY up a controller action

2008-10-31T20:52:05-07:00

My Neighbourhood "map_filter" action finds Property objects that match search criteria submitted by the user. It works, but I reckon it could be a bit shorter. Any suggestions?

# GET /neighbourhoods/map_filter
def map_filter
  @map_filter_errors  = nil # Stores errors related to a filter request.
  @properties         = nil # Stores the properties that match the filter constraints.

  # Extract the "accessible" attributes from the params.
  property_filter_attributes = AccessibleModelAttributes.extract PropertyFilter, params

  filtered_properties = PropertyFilter.new(property_filter_attributes).properties

  # If properties were returned.
  if filtered_properties[:properties]
    @properties = filtered_properties[:properties]

    @property_data = render_to_string(
      :partial  => 'properties/map_properties_table',
      :locals   => {:properties => @properties}
      ) 

    @number_of_properties_found_sentence = render_to_string :partial => 'properties/number_of_properties_found'
  else
    @map_filter_errors = filtered_properties[:errors]
  end
end

[Ruby] On DRYing/shortening form processing

2008-10-30T17:25:04-07:00

Hey Adam, thanks for the refactoring suggestion. It's a bit over my head at the moment, as I've never played with AR named scopes, and I'm not entirely sure what you meant the ItemFilter class to do.

What I ended up doing was moving #filtered_properties and #filter_* from the Property model into a new model, named PropertyFilter.

PropertyFilter inherits from ActiveRecord::BaseWithoutTable so that I can use AR's convenient validations, and not save data to a database table.

class PropertyFilter < ActiveRecord::BaseWithoutTable
  attr_accessible :price_direction, :price # ...many more attributes...

  column :price_direction,  :string
  column :price,            :integer
  column :bedrooms,         :integer
  # ...many more column definitions...

  valid_price_directions  = ['at most', 'at least']
  # ...many more valid values defined...

  validates_inclusion_of :price_direction,
    :in           => valid_price_directions,
    :allow_blank  => true
  # ...many more validations...

  def bedrooms_constraint
    return nil if self.bedrooms.blank?
    {:sql => 'bedrooms = :bedrooms', :bedrooms => self.bedrooms.to_s}
  end

  # ...many more _constraint methods...

  def properties
    # ...Call each _constraint method, and build the conditions for #find...

    return {:properties => Property.find(:all, :bounds => bounds, :conditions => [conditions_sql, condition_values])}
  end
end

[Ruby] DRYing/shortening form processing

2008-10-24T20:03:05-07:00

In my Rails app, the Property class method #filtered_properties accepts an arg containing params from an HTTP request, processes the params, and returns an array of properties matching the params.

I've created separate class methods (#filter_X , where "X" is "price", "bedrooms", etc) for processing each parameter, which #filtered_properties calls. However, #filtered_properties is still 120 lines.

Might you have any suggestions for how to make #filtered_properties and/or #filter_X more DRY and/or shorter?

  def self.filter_price(direction, price)
    return nil if direction.blank? or price.blank?
    return {:error => 'Invalid argument: direction'} unless ['at most', 'at least'].include? direction
    return {:error => 'Invalid argument: price'}     unless price.to_s =~ /\A[0-9]+\Z/

    case direction
      when 'at most'  then  return  {:sql => 'price <= :price', :price  => price.to_s}
      when 'at least' then  return  {:sql => 'price >= :price', :price  => price.to_s}
      else                  raise   'Unknown error'
    end
  end

  def self.filter_bedrooms(bedrooms)
    return nil if bedrooms.blank?
    return {:error => 'Invalid argument: bedrooms'} unless bedrooms.to_s =~ /\A[0-9]+\Z/

    {:sql => 'bedrooms = :bedrooms', :bedrooms => bedrooms.to_s}
  end

  #
  # ...Snip. There are several more methods like the ones above...
  #

  def self.filtered_properties(params)
    conditions_string = ''
    conditions_hash   = {}      # Stores the conditions for filtering out properties.
    condition_errors  = []      # Stores errors related to a filter request.

    # Process the "price" and "price_direction" params.
    filtered = filter_price params[:price_direction], params[:price]
    if filtered.blank?      # Do nothing.
    elsif filtered[:error]
      condition_errors << 'Please provide a number for the price. Eg: 1500'
    else
      conditions_string << filtered[:sql]
      conditions_hash[:price] = filtered[:price]
    end

    # Process the "bedrooms" param.
    filtered = filter_bedrooms params[:bedrooms]
    if filtered.blank?      # Do nothing.
    elsif filtered[:error]
      condition_errors << 'Please provide a number for the bedrooms. Eg: 2'
    else
      conditions_string << ' AND ' << filtered[:sql]
      conditions_hash[:bedrooms] = filtered[:bedrooms]
    end

    # Process the "bathrooms" param.
    filtered = filter_bathrooms params[:bathrooms]
    if filtered.blank?      # Do nothing.
      puts '> bathrooms is blank'
    elsif filtered[:error]
      condition_errors << 'Please provide a number for the bathrooms. Eg: 2'
    else
      conditions_string << ' AND ' << filtered[:sql]
      conditions_hash[:bathrooms] = filtered[:bathrooms]
    end

    # ...snip...

    return condition_errors unless condition_errors.empty?

    self.find :all, :conditions => [conditions_string, conditions_hash]

  end # End #filtered_properties