This can't be right

[Stefan Kroes]

Hi all,

I'm pretty new to Ruby and RoR but I'm trying to learn for a large
project. I wrote the following method in one of my models but it can't
be the right way to do this because ruby should be able to do it way
more concisely. Can you tell me of a better way?

def name
if !@name
property = content_properties.find_by_key('name')
if property
@name = property.value
else
@name = ""
end
else
@name
end
end

Thx in advance,

with regards,

Stefan Kroes

 

[Phlip]

Hi all,

I'm pretty new to Ruby and RoR but I'm trying to learn for a large
project. I wrote the following method in one of my models but it can't
be the right way to do this because ruby should be able to do it way
more concisely. Can you tell me of a better way?

Firstly, write unit tests covering each of its branches. That way, you can do
the edits I'm about to do, but one at a time, and pass all the tests after each one.

def name
if !@name
property = content_properties.find_by_key('name')
if property
@name = property.value
else
@name = ""
end
else
@name
end
end

Good design is about merging redundant things. Your @name appears too many
times, so we will have a crack at that.

def name
@name ||= ( property = content_properties.find_by_key('name')
property ? property.value : '' )
end

Yes, you can put a linefeed (or a ; ) inside parens. They return their last
statement's value, just like a function returns its last statement's value.

Next, ||= is a common idiom for "surprise assignment". If the left argument is a
nil or false variable, the right side is evaluated and assigned.

And, finally, @name is nil if it's not declared yet.

 

[Lyle Johnson]

I'm pretty new to Ruby and RoR but I'm trying to learn for a large
project. I wrote the following method in one of my models but it can't
be the right way to do this because ruby should be able to do it way
more concisely. Can you tell me of a better way?

def name
if !@name
property = content_properties.find_by_key('name')
if property
@name = property.value
else
@name = ""
end
else
@name
end
end

Here's one slightly shorter version:

def name
unless @name
property = content_properties.find_by_key('name')
@name = property ? property.value : ""
end
@name
end

Hope this helps,

Lyle

 

[Zundra Daniel]

[Note: parts of this message were removed to make it a legal post.]

property = content_properties.find_by_key('name') unless @name
property ? @name = property.value : @name = ""
@name

Is one way

 

[Simon Krahnke]

def name
if !@name
property = content_properties.find_by_key('name')
if property
@name = property.value
else
@name = ""
end
else
@name
end
end

Well, sick:

def name
@name ||= (property = content_properties.find_by_key('name') &&
property.name) || ""
end

mfg, simon .... untried

 

[Simon Krahnke]

Well, sick:

def name
@name ||= (property = content_properties.find_by_key('name') &&
property.name) || ""
end

Well, helper variables aren't "elegant":

def name
@name ||= (content_properties.find_by_key('name') ||
Struct.newname).new('')).name
end

mfg, simon .... trying's lame

 

[Peter Jones]

Well, helper variables aren't "elegant":

def name
@name ||= (content_properties.find_by_key('name') ||
Struct.newname).new('')).name
end

That's basically equivalent to:

,----
| def name
| @name ||= content_properties.find_or_initialize_by_key('name').name.to_s
| end
`----

or even this flame bait:

,----
| def name
| @name ||= content_properties.find_by_key('name').name rescue ""
| end
`----

Of course, I prefer the former.

 

[Robert Dober]

That's basically equivalent to:

,----
| def name
| @name ||= content_properties.find_or_initialize_by_key('name').name.to_s
| end
`----

or even this flame bait:

,----
| def name
| @name ||= content_properties.find_by_key('name').name rescue ""
| end
`----

Of course, I prefer the former.

I do not, because the former will not work when find_or_etc.etc. will return nil
Next we shall change the message from #name to #value and that done I
have no reason at all to flame ( there is no such thing on this list
you at all, I quite like the rescue, it is probable the most
readable solution for my eyes.

I see yet another alternative which is maybe not the most pretty code,
but maybe the easiest to deal with (debugging, evolution of the code)

@name ||= content_properties.find...("name")
@name &&= @name.value
@name ||= ""

just a completely different style.
Cheers
Robert

 

[Stefan Kroes]

I will try the last one first:

@name ||= ( property = content_properties.find_by_key('name') ?
property.value : "" )

Very short and very readable, all cases are in there nicely

Thx!

 

[Simon Krahnke]

I see yet another alternative which is maybe not the most pretty code,
but maybe the easiest to deal with (debugging, evolution of the code)

@name ||= content_properties.find...("name")
@name &&= @name.value
@name ||= ""

When @name is a String before that, first line does nothing, second line
throws an exception.

mfg, simon .... l

 

[Robert Dober]

* Robert Dober <[email protected]> (23:00) schrieb:

Guess I got it wrong

Well this is not an example for this coding style, sorry.
R.