How to simplify this structure ?

[JackTheRussel]

Hi.

I have program where user can give parameters on the command line.

Program includes lots of if, elsif, else clauses and it is quite messy. What I could do ?

example

#Here I have GetOpt module. User give parameters, and values goes to variables.
#example if user write -a 17 then variable $age=17

my $opt = new Getopt::Compact
    (name => 'program', 
     struct =>
   [[[qw(d days)], qq(start and end days), '=s', \my @days],
   [[qw(n name)], qq(persons name), '=s', \my $name],
   [[qw(a age)], qq(persons age), '=i', \$age],
   [[qw(l location)], qq(location), '=s', \my $location],
     etc...
    ]
    )->opts;

#Here I chech what parameters user has give
#Example if user gave -a parameter -> $parameters{age} = 1
#else $parameters{age} = 0 etc..

if (@days ne ""){$parameters{date} = 1; } else{$parameters{date} = 0; }

if ($name ne ""){$parameters{name} = 1; } else{$parameters{name} = 0; }

if ($age ne ""){$parameters{age} = 1; } else{$parameters{age} = 0; }

if ($location ne ""){$parameters{location} = 1; } else{$parameters{location} = 0; }
etc...

#Here I check how many parameters user has gave 
#If user didin't gave any parameters ->exit(0)

for my $key (keys(%parameters)){$total += $parameters{$key}}
if($total == 0)
{
exit(0)
};

# AND HERE WE START TO CREATE SQL-CLAUSES
#this is basic clause and we bond other clauses to this.

my $sql = SELECT something, something FROM TABBLE WHERE 1=1

#if name parameter has given bond this clause
if ($name ne ""){$sql .= " AND name = '$name'";}

#if age parameter has given bond this clause.

if ($age != ""){$sql .= " AND age = $age";} 

# if location parameter has given bond this clause.
if ($location ne ""){$sql .= " AND location = $location";}

AND SO ON.. Actually I have 15 parameters so 
in the end code is very,very messy.  

Do you have cure for this problem? I would be very pleased if somebody could help me!.

 

[MOrac]

Use iteration where it makes sense to do so. Your parameter list is already being fully definied and referenced in the struct you pass to GetOpt; use it. Instead of passing an anonymous array, store this somewhere real and use it to parse your data. For instance, if it becomes stored in $struct, then $struct->[$n] is the $n'th data entry item, where $struct->[$n]->[0]->[1] is the textual name, $struct->[$n]->[2] gives the data type and $struct->[$n]->[3] is a reference to the data. So, for instance:

foreach (@$struct) {
    $total += ($parameters{$_->[0]->[1]} = (${$_->[3]} ne "") ? 1 : 0);
    $sql .= (${$_->[3]} ne "") ? " AND $_->[0]->[1] = " . ($_->[2] eq "=s" ? "'" : "") . ${$_->[3]} . ($_->[2] eq "=s" ? "'" : "") : "";
}

This should do all the data mining you have currently shown.

 

[JackTheRussel]

MOrac.

I believe your solution is great and it works, but could you still give me larger example ?

I didin't fully understand the code you wrote.

 

[MOrac]

Your original code began:

my $opt = new Getopt::Compact
    (name => 'program',
     struct =>
   [[[qw(d days)], qq(start and end days), '=s', \my @days],
   [[qw(n name)], qq(persons name), '=s', \my $name],
   [[qw(a age)], qq(persons age), '=i', \$age],
   [[qw(l location)], qq(location), '=s', \my $location],
     etc...
    ]
    )->opts;

I proposed changing it to:

my $struct = [[[qw(d days)], qq(start and end days), '=s', \my @days],
   [[qw(n name)], qq(persons name), '=s', \my $name],
   [[qw(a age)], qq(persons age), '=i', \$age],
   [[qw(l location)], qq(location), '=s', \my $location],
     etc...
    ];
my $opt = new Getopt::Compact
    (name => 'program',
     struct => $struct
     )->opts;

This gives you the GetOpts structure in a reusable form, for later parsing. The first thing you then do is check each parameter into your hash $parameters as a true/false entry; you then do the same to build your $sql string. I replicate this by iterating though each element of the $struct GetOpts structure and checking each data reference. The line:

foreach (@$struct) {

iterates through the array reference $struct (hence the use of the form @$struct to refer to it) one element at a time. As I don't need to know which element has which index number in the array, it can be done this way. Each iteration places the array entry into the global default variable $_; if you would prefer to use something with a more natural name, name it explicitly in the foreach statement, viz:

foreach $item (@$struct) {

to place the iteration into $item. The first of these iterations, from the example, would give equivalence to:

$item           <==> [[qw(d days)], qq(start and end days), '=s', \my @days];
$item->[0]      <==> [qw(d days)];
$item->[0]->[0] <==> qw(d);
$item->[0]->[1] <==> qw(days);
$item->[1]      <==> qq(start and end days);
$item->[2]      <==> '=s';
$item->[3]      <==> \my @days;

Thus, to make things rather more readable than before, the proposed loop code would become:

foreach $item (@$struct) {
    local $data = ${$item->[3]}
    local $name = $item->[0]->[1];
    local $type = $item->[2];
    local $quote = "";
    if ($type eq "=s") {
        $quote = "'";
    }
    if ($data ne "") {
        $parameters{$name} = 1;
        $total += 1;
        $sql .= " AND $name = $quote$data$quote"
    } else {
        $parameters{$name} = 0;
    }
}

 

[MOrac]

Note that I appear to have missed a couple of terminating semicolons in the above code example. Whoops! You'll also of course need to keep the code that sets the original values to $total and $sql and follow the loop with the check on $total that gives the exit(0) if no parameters were set.

 

[JackTheRussel]

Thank you VERY much MOrac.

I start immediately to rebuild my program.