method parameters

[genericpenguin]

Hi guys

I have a quick one. When is this a bad idea?

class SOEClientConfig < SOEConfig
attr_accessor :set, :repository, :installedList
# SOEClientConfig.new(:repository)
def initialize(params)
@repository = params[:repository]
@set = params[:set]
@installedList = params[:installedList]
end
end

I find myself doing it cause I like keywords (I've been playing with
Smalltalk) and because it looks nice. But my tiny brain keeps beeping
that something is wrong. It feels a little loose. Like not failing
when I don't supply params. In some cases I do defaults like:

@repositoryLocalPath = (params[:repositoryLocalPath] ?
params[:repositoryLocalPath] : "/Library/WebServer/Documents/" +
@repositoryName + "/")

But I'm not sure. Any reasons why this would be a Bad Thing™?

[Michael Neale]

I would like to know too - just to be clear are you asking about
passing in a params map (simulating named params) versus explicit
params (which unfortunately are not named in ruby)?

(perhaps I mis-interpret ??)

Michael.

--
Michael D Neale
home: www.michaelneale.net
blog: michaelneale.blogspot.com

[Sven Schott]

Yes, that is exactly what I was asking. Thanks for clarifying that. On
second inspection, my post was not that clear. :)

[Julio Cesar Ody]

It's only evil when you don't document what keys exactly you can put
in that hash.

Apart from that, do it. Write some code with your method, see how it
looks. It if helps improving readability, and you're happy with it,
then it's the way to go.

I'd fear the preachers more than anything else.

[Michael Neale]

I think its really nice and readable, and in any case it seems to be
the convention throughout rails and many other ruby libs that I have
come across or used, so go with the flow !

(an annoying part of me kind of balks at the creation of a map for a
parameter, but that's premature my optimisation instinct kicking in -
excuse me while I slap it down).

So in summary, its kind of talking about:

your_method_here 1, "hai"

versus:

your_method_here :id => 1, :comment => "hai"

on the usage side. Obviously the latter is much nicer to read.
Interestingly when I used to use ruby as "a perl" (I never "got" perl,
so I would use ruby back in places when others would use perl - this
was long before rails) the latter didn't seem to be a common idiom,
but it is now.

[Gregory McIntyre]

If it were me, I'd write it like this:

class SOEClientConfig < SOEConfig
attr_accessor :set, :repository, :installedList
# SOEClientConfig.new(:repository)

def initialize(params=nil)
params ||= {}

@repository = params[:repository]
@set = params[:set]
@installedList = params[:installedList]
end
end

Hence "SOEClientConfig.new" is potentially valid (may use whatever
default values are in initialize).

--
Greg McIntyre

[Sven Schott]

Aah. That's clever. Pretty and functional. :)

[Gregory McIntyre]

2008/7/29 Sven Schott <sven....@gmail.com>:

> Aah. That's clever. Pretty and functional. :)

:)

I do defaults like this:

@repository = params[:repository] or "default value"

The only caveat is with booleans.

@is_repository = params[:is_repository] or true # always true, even
if false is passed explicitly

In which case, this might be more appropriate:

@is_repository = if params.key?(:is_repository) then
params[:is_repository] else true end

There might be a more succinct way to do this without resorting to the
? : operator (which I personally avoid for readability reasons).

And as Julio said, document the keys you accept. It makes people who
use your methods smile. ;-)

--
Greg McIntyre

[Sven Schott]

That is definitely simpler to read that the (?:) (which I happen to like :) ) but I get something funny when I try it:

 

irb(main):011:0> d = {}

=> {}

irb(main):012:0> g = d[:dontexist] or "default"

=> "default"

irb(main):013:0> g

=> nil

irb(main):014:0> g.class

=> NilClass

irb(main):015:0> 

 

But then I try this:

 

irb(main):017:0> g

=> nil

irb(main):018:0> d

=> {}

irb(main):019:0> g or d

=> {}

irb(main):020:0> d or g

=> {}

irb(main):021:0> a = d or g

=> {}

irb(main):022:0> a

=> {}

irb(main):023:0> d

=> {}

irb(main):024:0> g

=> nil

 

And i don't know what the hell is going on! d[:dontexist] in the first example and g in the second both return nil. So why the different behaviour? I hope I'm not doing something stupid.

[Julio Cesar Ody]

d = {}
g = (d[:dontexist] or "default")

That will work. Just add parentheses. Alternatively, you can use "||"
instead of "or" and avoid needing to use the parentheses altogether.

And by the way, no need for

def foo(bar)
bar ||= {}
end

Just do

def foo(bar = {})
...
end

Again, up to how you like to write your poetry.

[Gregory McIntyre]

2008/7/29 Julio Cesar Ody <juli...@gmail.com>:

> d = {}
> g = (d[:dontexist] or "default")
>
> That will work. Just add parentheses. Alternatively, you can use "||"
> instead of "or" and avoid needing to use the parentheses altogether.

Aaah yes. :-)

--
Greg McIntyre