Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Yaroslav Markin January 17th, 2009 @ 08:40 AM

  • Tag changed from “2.1.2, activerecord, mysql” to “2.1.2, activerecord, mysql, patch, tiny”
  • Assigned user set to “Pratik”

  Patch attached

  • 0001-ActiveRecord-Base-ordering-now-checking-if-order.patch 1.8 KB
• 

  Pratik January 18th, 2009 @ 03:46 AM

  • State changed from “new” to “wontfix”

  You shouldn't supply empty string :) It's not really designed to work with conditions and :joins, but it just does. It could easily break.

• You flagged this item as spam.
  

  Student January 18th, 2009 @ 03:33 PM

  Is this my payment for minimizing my example? :/

  I found this using generated code. With this functionality, I can do the following:

  @@@ruby cond_string = big computation order_string = bigger computation join_string = biggest computation

  Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)

  
  
  Without, I'm stuck with:
  
  @@@ruby
  cond_string = big computation
  order_string = bigger computation
  join_string = biggest computation
  
  options = {}
  options[:conditions] = cond_string unless cond_string.empty?
  options[:order] = order_string unless order_string.empty?
  options[:joins] = join_string unless join_string.empty?
  
  Venue.all(options)
  

  Unless I "really shouldn't" pass in an empty options string, either.

• You flagged this item as spam.
  

  Student January 18th, 2009 @ 03:36 PM

  That didn't format as advertised, and I don't seem to be able to edit my comment.

  Is this my payment for minimizing my example? :/

  I found this using generated code. With this functionality, I can do the following:

  cond_string = big computation order_string = bigger computation join_string = biggest computation

  Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)

  Without, I'm stuck with:

  cond_string = big computation order_string = bigger computation join_string = biggest computation

  options = {} options[:conditions] = cond_string unless cond_string.empty? options[:order] = order_string unless order_string.empty? options[:joins] = join_string unless join_string.empty?

  Venue.all(options)

  Unless I "really shouldn't" pass in an empty options string, either.

• You flagged this item as spam.
  

  Student January 18th, 2009 @ 03:41 PM

  Ick. Spaces matter.

  Is this my payment for minimizing my example? :/

  
  cond_string = big computation 
  order_string = bigger computation 
  join_string = biggest computation
  
  Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)
  

  Without, I'm stuck with:

  
  cond_string = big computation 
  order_string = bigger computation 
  join_string = biggest computation
  
  options = {} options[:conditions] = cond_string unless cond_string.empty? 
  options[:order] = order_string unless order_string.empty? 
  options[:joins] = join_string unless join_string.empty?
  
  Venue.all(options)
  

  Unless I "really shouldn't" pass in an empty options string, either.

• 

  Pratik January 18th, 2009 @ 06:06 PM

  
  order_string = bigger_computation
  order_string = nil if order_string.blank? 
  cond_string = big computation 
  join_string = biggest computation
  
  Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)
  

• You flagged this item as spam.
  

  Student January 18th, 2009 @ 08:47 PM

  There is currently no documentation on what happens if an option is passed which is nil or empty. I discovered this behavior (:joins, and :conditions work with blank parameters, :order does not) when using generated fragments. In my case, these generated fragments were themselves compounds, built with a final Array#join, thus always strings.

  I would like a clean, smooth, supported interface. Your solution would appear to be none of these.

• You flagged this item as spam.
  

  Yaroslav Markin January 18th, 2009 @ 08:53 PM

  @lifo while this ticket is far from important and I fully agree that @nil@-ifying parameter is just what Student needs, I think there is still sense in applying: right now there is a code snippet that checks for @nil@, we can just change it to @.present?@, that solves the problem just right.

• 

  Xavier Noria January 18th, 2009 @ 09:12 PM

  I am -1 here. The API shouldn't be designed to eat everything. In particular I don't really see the documentation stating that you can pass blank?s to :order (if it was really supported it should be documented).

  In my opinion :order has either a valid value or nothing. An empty string is an invalid value.

  Another possibility would be to clean the hash with a custom #compact method for hashes that removes entries with blank? values.

• You flagged this item as spam.
  

  Student January 19th, 2009 @ 03:33 AM

  I agree that this is not an error as in "the documentation says X will happen and it does not". But I believe that the observed behavior is not optimal, in that the caller is responsible (in some cases) to normalize data when there is only one reasonable way to interpret the data.

  Certainly, the caller can, as I currently must, ensure that no option is set with blank parameters. But if the purpose of a framework is to reduce the work of the using programmer, this would seem to be a strange way to proceed.