alternatives for branching

C

ccc31807

In a web application that I maintain, the main logic contains several
dozen elsif statements nested to several levels. The tests aren't all
the same, although in the code sample I've posted below they are the
same. I can maintain this fairly easily but I'd like to redo the logic
as it seems unwieldy and ugly to me.

This is a database application, and each branch typically calls two
procedures, the first running an SQL statement and the second a
display routine.

Is there a Perl alternative to this logic? If so, how could this be
rewritten in Perl? In general, what are alternatives to a whole bunch
of elsif statements nested to several levels?

Thanks, CC.

-------code sample------------------------
elsif ($subpage =~ /ResetPW/)
{
my $email_hash = SQL::reset_password($foruser);
CONSOLE::successful_email("$email_hash->{fullname}",
"$email_hash->{email1},$email_hash->{email2},$email_hash->{email3}",
'(e-mail address removed)', '', 'Your MUPD password has been reset',
"Dear $email_hash->{fullname}: Yoursite.org password has been reset to
********'. Please use the contact form if you have any questions about
this. Thank you.");
HTML::control_button($sesskey, 'Administer Security');
}
elsif ($subpage =~ /ChangeRole/)
{
SQL::change_role($foruser);
HTML::control_button($sesskey, 'Administer Security');
}
#-----------calendar (events) logic-----------------
elsif ($subpage =~ /DeleteEvent/)
{
SQL::delete_event($foruser);
HTML::control_button($sesskey, 'Manage Events');
}
elsif ($subpage =~ /EditEvent/)
{
HTML::control_button($sesskey, 'Manage Events');
HTML::manage_event_form($sesskey, $foruser);
}
elsif ($subpage =~ /UpdateEvent/)
{
SQL::update_event($calid, $event, $description, $place,
$eventdate, $type, $uniform, $required, $display, $comments);
HTML::control_button($sesskey, 'Manage Events');
}
elsif ($subpage =~ /DisplayEvents/)
{
my $events_ref = SQL::get_events();
HTML::display_events($events_ref);
}
#-----------news items logic-----------------
elsif ($subpage eq 'InsertNews')
{
SQL::insert_news($head, $first, $body, $priority,
$display, $comments, $username);
my $msg = qq(
HEADLINE: $head
FIRST: $first
BODY: $body
COMMENTS: $comments
USERNAME: $username
);
SITE::notify_admin('News Article Added',$msg);
HTML::control_button($sesskey, 'Add News');
}
elsif ($subpage =~ /DeleteNews/)
{
SQL::delete_news($foruser);
HTML::control_button($sesskey, 'Manage News');
}
elsif ($subpage =~ /EditNews/)
{
HTML::manage_news_form($sesskey, $foruser);
HTML::control_button($sesskey, 'Manage News');
}
elsif ($subpage =~ /UpdateNews/)
{
SQL::update_news($newsid, $head, $first, $body, $priority,
$display, $comments);
HTML::control_button($sesskey, 'Manage News');
}
elsif ($subpage =~ /DisplayNews/)
{
my $news_ref = SQL::get_news();
HTML::display_news($news_ref);
}
 
J

John Bokma

ccc31807 said:
In a web application that I maintain, the main logic contains several
dozen elsif statements nested to several levels. The tests aren't all
the same, although in the code sample I've posted below they are the
same. I can maintain this fairly easily but I'd like to redo the logic
as it seems unwieldy and ugly to me.

I used Regexp::Assemble in the past for that, but my code broke when I
moved to Perl 5.10.

If the names are all constant strings, and you don't do actuall
matching use something like:

my %dispatch = (

ResetPw => \&reset_pw,
ChangeRole => \&change_role,
:
);

my $sub = $dispatch{ $subpage };
defined $sub or $sub = ....
$sub->( ... );
 
S

Skye Shaw!@#$

In a web application that I maintain, the main logic contains several
dozen elsif statements nested to several levels.

<snip if else blocks>

Your conditions focus on several different nouns: News, Event, etc...
with similar actions -create, read, update, etc...

Most likely these nouns actually represent distinct entities on your
web site. If so, the most logical thing to do would be to handle
requests to delete News in a script (controller) used solely for
News.

Same goes for Event, Role, and whatnot.

-Skye
 
J

Jochen Lehmeier

Is there a Perl alternative to this logic?
elsif ($subpage =~ /ChangeRole/)
{
SQL::change_role($foruser);
HTML::control_button($sesskey, 'Administer Security');
}

You got some pointers already.

Two concrete techniques (just examples, not finished code, not necessarily
well-written, and you will find people here who frown upon either
</disclaimer> ) are:


my %hash = (
ChangeRole => sub { SQL::change_role($foruser); ... },
DeleteEvent => sub { ... },
...
);

{
no strict "refs";
$hash{$subpage}->();
}


And:


sub ChangeRole
{
SQL::change_role...
}

sub DeleteEvent
{
...
}

{
no strict "refs";
$subpage->();
}


Just to get your started (there are many different ways to go about this).

There are also modules on CPAN for MVC frameworks, if you want to pull out
the big guns.
 
J

Jürgen Exner

ccc31807 said:
In a web application that I maintain, the main logic contains several
dozen elsif statements nested to several levels. The tests aren't all
the same, although in the code sample I've posted below they are the
same. I can maintain this fairly easily but I'd like to redo the logic
as it seems unwieldy and ugly to me.

This is a database application, and each branch typically calls two
procedures, the first running an SQL statement and the second a
display routine.

Is there a Perl alternative to this logic? If so, how could this be
rewritten in Perl? In general, what are alternatives to a whole bunch
of elsif statements nested to several levels?

I think you are looking for a 'dispatch table'.

jue
 
C

ccc31807

My first response would be 'see Catalyst'. I'm certain it can be made to
do what you want with your URLs fairly easily, though it would require
completely rewriting the app (splitting it into multiple parts, so that
each 'elsif' branch below becomes a method in a controller class).

Ben, there are many web frameworks, and after experimenting with
several I've kinda given up in disgust. You spend a lot of effort
learning the framework (in my case anyway) and then get frustrated by
trying to make it do things you can't persuade it to do. I find it
easier to create my own framework with just the functionalities I
need, and I don't especially want to rewrite the applications.
The usual alternatives to a whole lot of elsifs are either a dispatch
table or an object with methods (and now, of course, we have given/when,
though I'm not yet convinced how much it buys you). It's not clear below
whether you need those matches to be matches, or whether you can extract
the interesting bit of $subpage and use it as a key to a hash or a
method name.

I have read through but not digested both Conways OOPerl and Dominus'
HOP. I have some experience writing OO apps in Java (mostly), C++, and
Lisp, but for some reason have had a lot of trouble applying these
lessons to Perl. I really like HOP in general, and the dispatch table
(chapter 1 or 2) appeals to me. I use hashes almost exclusively to
initialize environmental variables for my scripts that use then, but
these are true variables, not behaviors.

Maybe I'll experiment and learn a new way New for me) of doing things.
In general, if your conditions are truly arbitrary there is no
alternative to a great big control structure, so you need to find
subsets of conditions which are 'the same' and turn them into dispatch
tables. If your logic is complicated you may find you need a much
shorter if/elsif construction which simply chooses which table to
dispatch on and where to get the key from.

Okay. Let me ask this. Is it better to encapsulate your functions in a
sub and call the sub or to call the functions directly in some kind of
way? How do I handle the issue of passing the correct arguments to the
functions? (The parameters are read into global variables because of
how the HTTP form elements are sent to the script.)

CC
 
J

Jochen Lehmeier

Is it better to encapsulate your functions in a
sub and call the sub or to call the functions directly in some kind of
way?

What do you mean by "function"? Do you mean the payload, i.e. the calls
from inside the "if {}" blocks in your example? Best keep giving us
examples of what you mean.

Anyway, whatever you do, I guess it's mostly a question of what looks good
to yourself. Unless you get lots of requests per second, performance will
not differ much between the alternatives. If I understood you correctly,
you're not about to create a great new framework, but just trying to
organise your code a bit more maintainable.
How do I handle the issue of passing the correct arguments to the
functions?

As you're in a CGI setting, your arguments are all named (i.e., key-value
pairs)? If so, then passing them as a hash seems appropriate.

For example:


sub function
{
my %args = @_;

print "name is ".$args{name};
}

function(name => "xyz");


(Remember, an array, if interpreted as a hash, works just fine; adjancent
pairs are transformed into key+value).

Or, if that array-as-hash thing bothers, you, feel free to:


sub function
{
my ($args)=@_;

print "name is ".$args->{name};
}

function({name => "xyz"});


Again, as before, there are lots of different ways to handle this. For
example, the CGI library allows you to pass in either named arguments (as
in my first example) or unnamed positional arguments, or even a mixture,
by using the convention that the named arguments begin with a "-" in their
name, and some additional parsing. Check out "perldoc -m CGI" if you're
interested.
 
S

sln

In a web application that I maintain, the main logic contains several
dozen elsif statements nested to several levels. The tests aren't all
the same, although in the code sample I've posted below they are the
same. I can maintain this fairly easily but I'd like to redo the logic
as it seems unwieldy and ugly to me.

This is a database application, and each branch typically calls two
procedures, the first running an SQL statement and the second a
display routine.

Is there a Perl alternative to this logic? If so, how could this be
rewritten in Perl? In general, what are alternatives to a whole bunch
of elsif statements nested to several levels?

Thanks, CC.

Typically you could make a package out of it.
The biggest problem is not the branching, its the organization
of your data. How do you blindly know what to set the data
before even branching?

You need to figure out the data handling before making a package.
Below lays out organized data for that decision.

-sln


use strict;
use warnings;

## Globals
#
my %Packet = (
'sesskey' => '',
'foruser' => '',
'calid' => '',
'event' => '',
'description' => '',
'place' => '',
'eventdate' => '',
'type' => '',
'uniform' => '',
'required' => '',
'display' => '',
'comments' => '',
'newsid' => '',
'username' => '',
'head' => '',
'first' => '',
'body' => '',
'priority' => '',
'result' => '',
);
my @SubNames =
qw( ResetPW ChangeRole DeleteEvent EditEvent
UpdateEvent DisplayEvents ^InsertNews$ DeleteNews
EditNews UpdateNews DisplayNews);

my $rxhandler = join '|', @SubNames;

my %Handler = (
resetpw => \&resetPW,
changerole => \&changeRole,
deleteevent => \&deleteEvent,
editevent => \&editEvent,
updateevent => \&updateEvent,
displayevents => \&displayEvents,
insertnews => \&insertNews,
deletenews => \&deleteNews,
editnews => \&editNews,
updatenews => \&updateNews,
displaynews => \&displayNews,
);

# -----------------------------------
# Could be external if made into package
#
my $subpage = 'action: ResetPW';

getSub($subpage)->(\%Packet);

exit (0);
# -----------------------------------

## Handlers ...
#
sub getSub {
my ($parse_str) = @_;
$parse_str =~ /($rxhandler)/ && return $Handler{lc $1};
die "Cannot find handler for '$parse_str'"
}

sub resetPW {
my ($P) = @_;
# debug
print "In ResetPW\n" and return;
#
my $email_hash = SQL::reset_password( $P->{foruser} );
CONSOLE::successful_email(
"$email_hash->{fullname}",
"$email_hash->{email1},$email_hash->{email2},
$email_hash->{email3}",
'(e-mail address removed)',
'',
'Your MUPD password has been reset',
"Dear $email_hash->{fullname}: Yoursite.org password has been reset to ********'.
Please use the contact form if you have any questions about this. Thank you."
);
HTML::control_button( $P->{sesskey}, 'Administer Security' );
$P->{result} = 'OK'
}

sub changeRole {
my ($P) = @_;
SQL::change_role( $P->{foruser} );
HTML::control_button( $P->{sesskey}, 'Administer Security' );
$P->{result} = 'OK'
}

# calendar event handlers
# ..
sub deleteEvent {
my ($P) = @_;
SQL::delete_event( $P->{foruser} );
HTML::control_button( $P->{sesskey}, 'Manage Events' );
$P->{result} = 'OK'
}

sub editEvent {
my ($P) = @_;
HTML::control_button( $P->{sesskey}, 'Manage Events' );
HTML::manage_event_form( $P->{sesskey}, $P->{foruser} );
$P->{result} = 'OK'
}

sub updateEvent {
my ($P) = @_;
SQL::update_event(
$P->{calid}, $P->{event}, $P->{description}, $P->{place},
$P->{eventdate}, $P->{type}, $P->{uniform}, $P->{required},
$P->{display}, $P->{comments} );
HTML::control_button( $P->{sesskey}, 'Manage Events' );
$P->{result} = 'OK'
}

sub displayEvents {
my ($P) = @_;
my $events_ref = SQL::get_events();
HTML::display_events( $events_ref );
$P->{result} = 'OK'
}

# news item handlers
# ..
sub insertNews {
my ($P) = @_;
SQL::insert_news(
$P->{head}, $P->{first}, $P->{body}, $P->{priority},
$P->{display}, $P->{comments}, $P->{username} );
my $msg = qq(
HEADLINE: $P->{head}
FIRST: $P->{first}
BODY: $P->{body}
COMMENTS: $P->{comments}
USERNAME: $P->{username}
);
SITE::notify_admin( 'News Article Added', $msg );
HTML::control_button( $P->{sesskey}, 'Add News');
$P->{result} = 'OK'
}

sub deleteNews {
my ($P) = @_;
SQL::delete_news( $P->{foruser} );
HTML::control_button( $P->{sesskey}, 'Manage News' );
$P->{result} = 'OK'
}

sub editNews {
my ($P) = @_;
HTML::manage_news_form( $P->{sesskey}, $P->{foruser} );
HTML::control_button( $P->{sesskey}, 'Manage News' );
$P->{result} = 'OK'
}

sub updateNews {
my ($P) = @_;
SQL::update_news(
$P->{newsid}, $P->{head}, $P->{first}, $P->{body},
$P->{priority}, $P->{display}, $P->{comments} );
HTML::control_button( $P->{sesskey}, 'Manage News' );
$P->{result} = 'OK'
}

sub displayNews {
my ($P) = @_;
my $news_ref = SQL::get_news();
HTML::display_news( $news_ref );
$P->{result} = 'OK'
}
__END__
 
C

ccc31807

Typically you could make a package out of it.
The biggest problem is not the branching, its the organization
of your data. How do you blindly know what to set the data
before even branching?

The other side of this contains logic very similar to this, something
like this (from memory):
HTML::get_header($var1, $var2);
HTML::get_banner($var3, $var4);
HTML::get_content($page);
HTML::get_footer();
exit(0);

The get_content($page) sub gets the content from the database and
displays the page, depending on the $page argument.

As to the data, I get ALL of the data from the user's browser set as
lexical variables to the package. The top of the application has lines
like this:
use CGI qw:)standard);
my $page = param('page');
my $subpage = param('subpage');
my $var1 = param('var1'');
etc ...
You need to figure out the data handling before making a package.
Below lays out organized data for that decision.

I'm going to have to spend some time looking at this. The application
works fine and isn't difficult to maintain, so I don't have an
incentive to rewrite the entire thing. I would rather change this
incrementally over time.

I'd really like to write a script that will generate the proper code
given the source listings as data, but this is a project for a rainy
day.

Thanks for your suggestion, I really appreciate it, CC.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,770
Messages
2,569,583
Members
45,073
Latest member
DarinCeden

Latest Threads

Top