Ross Bamford
2/2/2006 8:43:00 AM
On Thu, 2006-02-02 at 13:17 +0900, Michael Judge wrote:
> Ross Bamford wrote:
> > On Thu, 2006-02-02 at 09:57 +0900, Michael Judge wrote:
> >
> >> 1. Storing executable code in the database is a security problem
> >> 2. instance_eval is slow
> >> 3. The alternative (a big if/elsif tree) would span many pages and
> >> be unweildy.
> >>
> >> Have a better suggestion?
> >
> > Not necessarily better, but how about something like:
> >
> > class Commands
> > [snip]
> > def dispatch(line)
> > if line =~ /([QXM])-?(.*)/
> > send($1.intern, $2)
> > else
> > raise "Invalid input: #{line}"
> > end
> > end
> > end
>
> That's neat, Ross. I wasn't familiar with the send command. Looks like
> the consequence is that the grammar has to fit an easy regular
> expression or you'd be duplicating it in the match and again in the
> method definition... Well, that's not necessarily a bad thing.
> Consistency is good too.
>
Agreed, but it did bug me a bit, too :) Depending on the actual input
format you could optimise that away though I think, e.g.
class Commands
def M(args)
# Notice that regexp here is now responsible for
# for handling the '-' after the initial letter.
if args =~ /^-(\d+) (.*)$/
puts " [ ] #{$2}"
end
end
def dispatch(line)
begin
send(line.slice!(0,1), line)
rescue NoMethodError
raise "Invalid input: #{line}"
end
end
end
That way you're doing only one match per dispatch, and validating
implicitly (Ruby will raise NoMethodError if the command is bad). Since
we're forcing only a single letter, it shouldn't be possible for people
to input e.g. 'exit-666 1' or something to breach security.
A win with this approach I think is that it keeps everything where it
should be, i.e. the commands themselves are responsible for processing
their arguments, however they see fit. Also, you can easily add new
commands at runtime, simply by definining a new method. There's no
'command registry' anywhere.
One other change I'd make to my previous post would be to make the
command methods private.
> What do programmers normally do when they have a case statement that's
> 30 or more items long? Previously I've just left it as a case statement
> and spent the life of the project ticked at it.
>
Ordinarily I think I'd consider it a code smell (or maybe a "design
smell"?). Maybe I'd fix it, maybe not, but like you I'd at least _want_
to :).
--
Ross Bamford - rosco@roscopeco.REMOVE.co.uk