[lnkForumImage]
TotalShareware - Download Free Software
Confronta i prezzi di migliaia di prodotti.
Asp Forum
 Home | Login | Register | Search 


 

Forums >

comp.lang.ruby

Rather validate values or use exceptions?

Joshua Muheim

6/18/2008 7:26:00 PM

Hi all

I'm working on a little Rails application and have a general coding
question.

So far my looks like the following:

---
music_artist_id = params[:booking_ticket_music_artist_id]

if music_artist_id and MusicArtist.exists?(music_artist_id)
@music_artist = MusicArtist.find(music_artist_id)
page[:mini_profile].show.reload
else
page[:mini_profile].hide
end
---

On the first line I'm checking whether an optional parameter is given or
not. This works OK, but I wonder whether it is "better code" doing the
following?

---
begin
@music_artist =
MusicArtist.find(params[:booking_ticket_music_artist_id])
page[:mini_profile].show.reload
rescue RecordNotFound
page[:mini_profile].hide
end
---

When looking at it, the second one looks cleaner to me, but what would
you say? Maybe the first one is quite faster than the second one?

Thanks for your opinion,
Josh
--
Posted via http://www.ruby-....
11 Answers

Eleanor McHugh

6/18/2008 8:51:00 PM

0

On 18 Jun 2008, at 20:26, Joshua Muheim wrote:
> I'm working on a little Rails application and have a general coding
> question.
>
> So far my looks like the following:
>
> ---
> music_artist_id = params[:booking_ticket_music_artist_id]
>
> if music_artist_id and MusicArtist.exists?(music_artist_id)
> @music_artist = MusicArtist.find(music_artist_id)
> page[:mini_profile].show.reload
> else
> page[:mini_profile].hide
> end
> ---
>
> On the first line I'm checking whether an optional parameter is
> given or
> not. This works OK, but I wonder whether it is "better code" doing the
> following?
>
> ---
> begin
> @music_artist =
> MusicArtist.find(params[:booking_ticket_music_artist_id])
> page[:mini_profile].show.reload
> rescue RecordNotFound
> page[:mini_profile].hide
> end
> ---
>
> When looking at it, the second one looks cleaner to me, but what would
> you say? Maybe the first one is quite faster than the second one?

I'd tend to use the second version but conceivably performance gains
would accrue with the former approach as you avoid a whole class of
unnecessary database lookups. As to whether or not that matters
depends on the application...


Ellie

Eleanor McHugh
Games With Brains
http://slides.games-with-...
----
raise ArgumentError unless @reality.responds_to? :reason

ara.t.howard

6/18/2008 8:58:00 PM

0


On Jun 18, 2008, at 1:26 PM, Joshua Muheim wrote:

> When looking at it, the second one looks cleaner to me, but what would
> you say? Maybe the first one is quite faster than the second one?

the first method is a classic race condition. if you care about
things like that use the second.

a @ http://codeforp...
--
we can deny everything, except that we have the possibility of being
better. simply reflect on that.
h.h. the 14th dalai lama

Joshua Muheim

6/18/2008 9:08:00 PM

0

ara.t.howard wrote:
> On Jun 18, 2008, at 1:26 PM, Joshua Muheim wrote:
>
>> When looking at it, the second one looks cleaner to me, but what would
>> you say? Maybe the first one is quite faster than the second one?
>
> the first method is a classic race condition. if you care about
> things like that use the second.
>
> a @ http://codeforp...

After googling for Race Condition I guess I know now what this should
be, but I don't know how to avoid it...? Any suggestions? :-)
--
Posted via http://www.ruby-....

ara.t.howard

6/18/2008 9:10:00 PM

0


On Jun 18, 2008, at 1:26 PM, Joshua Muheim wrote:

realized i should have been a bit more clear, you have at least two
race conditions here


1)

> if music_artist_id and MusicArtist.exists?(music_artist_id)

record found, but deleted by another process

>
> @music_artist = MusicArtist.find(music_artist_id)

raises RecordNotFound


2)

> if music_artist_id and MusicArtist.exists?(music_artist_id)

record claims no to exist

another process creates it right here

>
> @music_artist = MusicArtist.find(music_artist_id)

so we never execute this



a @ http://codeforp...
--
we can deny everything, except that we have the possibility of being
better. simply reflect on that.
h.h. the 14th dalai lama

ara.t.howard

6/18/2008 9:11:00 PM

0


On Jun 18, 2008, at 3:08 PM, Joshua Muheim wrote:

> After googling for Race Condition I guess I know now what this should
> be, but I don't know how to avoid it...? Any suggestions? :-)

try to find it, if it doesn't race an exception you're all good.
rescue the exception when not found if that makes sense. in
otherwords basically your second approach.

a @ http://codeforp...
--
we can deny everything, except that we have the possibility of being
better. simply reflect on that.
h.h. the 14th dalai lama

Robert Dober

6/18/2008 9:34:00 PM

0

On Wed, Jun 18, 2008 at 11:11 PM, ara.t.howard <ara.t.howard@gmail.com> wro=
te:
>
> On Jun 18, 2008, at 3:08 PM, Joshua Muheim wrote:
>
>> After googling for Race Condition I guess I know now what this should
>> be, but I don't know how to avoid it...? Any suggestions? :-)
>
> try to find it, if it doesn't race an exception you're all good. rescue =
the
> exception when not found if that makes sense. in otherwords basically yo=
ur
> second approach.
>
yes maybe it will become clearer to you if you imagine that in your
first approach you would need to lock the table before executing your
if statement, and that of course is a dangerous, resource consuming
approach.

The second approach just uses one single atomic (at least at the
abstraction level we are operating here) acess.

Cheers
Robert




--=20
http://ruby-smalltalk.blo...

---
Les m=EAmes questions qu'on se pose
On part vers o=F9 et vers qui
Et comme indice pas grand-chose
Des roses et des orties.
-
Francis Cabrel

Joshua Muheim

6/18/2008 9:37:00 PM

0

Thanks to you all, guys, always happy to learn something new :-)
--
Posted via http://www.ruby-....

Sam Smoot

6/19/2008 1:25:00 AM

0

Yes, the second example. Because not finding the record for a known
key is "exceptional".

*But* I'd like to say that this is a minority rule. Generally
speaking:

* Exceptions should *NOT* be used for flow-control
* Exceptions should be reserved for exceptional situations

People always rail against these, but they're always wrong. ;-) throw/
catch will give you most of the benefits, without the draw-backs.
Namely:

Exceptions are _orders of magnitude slower_ than normal flow-control
structures.

There's probably languages these guidelines don't apply to, but Ruby/
MRI isn't one of them. ;-)

-Sam

ara.t.howard

6/19/2008 5:14:00 AM

0


On Jun 18, 2008, at 7:23 PM, Sam Smoot wrote:

> Exceptions are _orders of magnitude slower_ than normal flow-control
> structures.

only when they occur - having a rescue block defined cost next to
nothing on most cases. the best approach is often to *try* to use
conditional structures but to still use the rescue block - but that
ends up being very verbose and i'm personally no fan of the common
rails pattern of using AR in such a fashion that the 'db ought to
maintain integrity'.

so while i totally agree with you i also see *way* too much ruby code
written as

unless File.exists?( pidfile )
# race condition
create_pidfile pidfile

which races on a normal fs and races *badly* on standard production
shared filesystems like NFS. so, although exceptions might be slow,
i'll take them over code that works 'most of the time' any day -
although, if code is going to blow up i do like it to do so order of
magnitude more quickly ;-)

just my two cents on the issue - not responding to your post
directly. i guess i would summarize by saying that i totally agree,
but that there are *far* more exceptional behaviours in the real world
that much code seems to be acknowledge.

cheers.

a @ http://codeforp...
--
we can deny everything, except that we have the possibility of being
better. simply reflect on that.
h.h. the 14th dalai lama

Robert Dober

6/19/2008 5:58:00 AM

0

On Thu, Jun 19, 2008 at 3:23 AM, Sam Smoot <ssmoot@gmail.com> wrote:

Another point might be which exceptions to rescue!
I once thought to be very smart and ducktypy by catching NameError
instead of checking for #respond_to?
The code really looked quite nice

def whatever
o =3D some_way_to_get_it
message =3D some_other_way_to_get_it
o.send message
rescue NameError
say_something_not_so_nice
end

But I had a mistype deep in the call stack of some_way_to_get_it. Was
one of the worst debugging session I ever had.

Cheers
Robert

--=20
http://ruby-smalltalk.blo...

---
Les m=EAmes questions qu'on se pose
On part vers o=F9 et vers qui
Et comme indice pas grand-chose
Des roses et des orties.
-
Francis Cabrel