Rather validate values or use exceptions?

[Joshua Muheim]

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

 

[Eleanor McHugh]

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

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-brains.net

 

[ara.t.howard]

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://codeforpeople.com/

 

[Joshua Muheim]

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

a @ http://codeforpeople.com/

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?

 

[ara.t.howard]

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://codeforpeople.com/

 

[ara.t.howard]

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://codeforpeople.com/

 

[Robert Dober]

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.blogspot.com/

---
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]

Thanks to you all, guys, always happy to learn something new

 

[Sam Smoot]

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]

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://codeforpeople.com/

 

[Robert Dober]

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.blogspot.com/

---
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

 

[Sam Smoot]

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://codeforpeople.com/

No, you're right, agree 100%.

I'm more thinking of seeing people raise/catch exceptions for
validation. Or a locked record in the database. Basically using them
like a GOTO. And they're really functionally identical to throw/catch
in those situations, because you don't _really_ care about the stack-
trace that produced a validation error. It's a case you can handle.

On the other hand, ArgumentError is my favorite thing since sliced
bread. ;-) No, I can't pass a Symbol, String, Integer, Array or Hash.
Pick one. Even if I have to type a couple more characters. At least I
won't forget the method signature.

/off topic