[Ruby] code review

[Ofer Matan]

The comments have been very useful. Ben's pointer to with_scope would allow
me to rewrite self.masters easily and
viewing the source code for with_scope has been very educational.

Ryan your comment is what I am looking for:

options[:conditions] = Array(options[:conditions])

is clever in that it  makes sure the string and array forms are in canonical
form - an array.

Question:
One reason I used defaults_options is so not to change the parameters that
are passed into the method - this is one
of the hardest things for me to get used to in Ruby - everything is passed
by reference. If you are fiddling around with parameters what is the right
way to go about it?

-Ofer

------------------------------------------------
On Oct 29, 2006, at 5:32 PM, Ofer Matan wrote:

> The following code is a class method for an
> ActiveRecord that gets all objects with a particular
> filter, but I wanted it also to allow all the other
> fancy find options that ActiveRecord provides:
>
>   MASTER_ID_IS_NULL = " and master_id is NULL"
>   def self.masters(options={})
>     default_options = {:conditions => 'true'}
>     options = default_options.merge(options)
>     if options[:conditions].is_a?(Array) then
>       options[:conditions][0] += MASTER_ID_IS_NULL
>     else #string
>       options[:conditions] += MASTER_ID_IS_NULL
>     end
>     find(:all,options)
>   end

this seems like an awful lot of work to wrap up one find and make
sure that master_id is null. I'd look at the find_all_with_XXX
methods and see how much you're actually gaining by having this code.
That said, a couple suggestions:

+ get rid of the default_options variable. It doesn't really help
readability.

+ try:

options[:conditions] = Array(options[:conditions])
options[:conditions][0] += MASTER_ID_IS_NULL