Rather validate values or use exceptions?

Ruby
1

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

2

On 18 Jun 2008, at 20:26, Joshua M. wrote:

page[:mini_profile].show.reload

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

raise ArgumentError unless @reality.responds_to? :reason

3

On Jun 18, 2008, at 1:26 PM, Joshua M. 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://codeforpeople.com/

4

ara.t.howard wrote:

On Jun 18, 2008, at 1:26 PM, Joshua M. 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://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? :slight_smile:

5

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

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

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

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/

6

On Jun 18, 2008, at 3:08 PM, Joshua M. 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? :slight_smile:

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/

7

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

8

On Wed, Jun 18, 2008 at 11:11 PM, ara.t.howard [email protected]
wrote:

On Jun 18, 2008, at 3:08 PM, Joshua M. 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? :slight_smile:

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.

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

–
http://ruby-smalltalk.blogspot.com/

Les mêmes questions qu’on se pose
On part vers où et vers qui
Et comme indice pas grand-chose
Des roses et des orties.

Francis Cabrel

9

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. :wink: 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. :wink:

-Sam

10

On Jun 18, 2008, at 7:23 PM, Sam S. 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 :wink:

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/

11

On Thu, Jun 19, 2008 at 3:23 AM, Sam S. [email protected] 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 = some_way_to_get_it
message = 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

–
http://ruby-smalltalk.blogspot.com/

Les mêmes questions qu’on se pose
On part vers où et vers qui
Et comme indice pas grand-chose
Des roses et des orties.

Francis Cabrel

12

On Jun 19, 12:14 am, “ara.t.howard” [email protected] wrote:

maintain integrity’.
i’ll take them over code that works ‘most of the time’ any day -
a @http://codeforpeople.com/

we can deny everything, except that we have the possibility of being
better. simply reflect on that.
h.h. the 14th dalai lama

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. :wink: 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. :slight_smile:

/off topic