monk2b has asked for the wisdom of the Perl Monks concerning the following question:

I am parsing a string and would like to make conditional decisions based on data the stings contains. Could you guys tell me how effective this code is in doing that and if there is a better way I would certainly appreciate knowing that too.

I want to keep the code as easy to read and maintain as possible too. 
if(!defined($mystring)){
    dosumpton("nogames",undef,undef,undef);
} elsif ($mystring){
    ($major,$minor,$bld,$rev) = split(/\./, $mystring);
    if($major){
        if($minor){
            if($bld){
                if($rev){
                    dosumpton($major,$minor,$bld,$rev);
                } else {
                dosumpton($major,$minor,$bld,undef);
                }
            } else {
                dosumpton($major,$minor,undef,undef);
            }
        } else {
            dosumpton($major,undef,undef,undef);
        }
    }
} else {
    # Something went wrong and we should be notified about it
}

[download]

Replies are listed 'Best First'.
Re: Are there corner cases I am missing
by kennethk (Abbot) on Nov 14, 2011 at 16:55 UTC
    Given that you are always using dosumpton, it would seem more logical to me to explicitly treat an argument list and pass that to your subroutine. Is there some reason you need to explicitly pad your argument list to 4 terms? If not, then your script is functionally very close to: 
    if($mystring){
    dosumpton(split(/\./, $mystring));
} elsif (not defined $mystring){
    dosumpton("nogames");
} else {
    # Something went wrong and we should be notified about it
}

    [download]

    The way in which these two forms differ is the question of trailing false but defined values - zeros ('0') and empty strings (''). Your clobber tailing falses with undefs could be reproduced by swapping line 2 above with: 

    my @args = split /\./, $mystring;
pop @args while (@args and !$args[-1]);
dosumpton(@args);

    [download]

    Finally, in a very literal sense, your existing branching fails to handle the case where the parent string is defined but major is false - for example, if your version number were 0.11. That will fall right through your block.

[reply]
[d/l]
[select]
Re: Are there corner cases I am missing
by jethro (Monsignor) on Nov 14, 2011 at 16:58 UTC

    How about this:

    ...
   @all = split(/\./, $mystring);
   dosumption(@all);

    [download]

    Assuming your script handling of "0" and "" as if it was undef was an accident

[reply]
[d/l]
Re: Are there corner cases I am missing
by choroba (Cardinal) on Nov 14, 2011 at 16:54 UTC
    What should the code do? If the version string is 5.14.0, you are replacing the build value(?) with an undef. You should rather test for defined. But, if your intention is to supply undefs for missing information at the end of the string, you can delete most of the code: for 5.14.0, revision is already undef.
[reply]
[d/l]
[select]
Re: Are there corner cases I am missing
by monk2b (Pilgrim) on Nov 14, 2011 at 18:02 UTC
    Sorry Guys, I can see my question and code was not clear at all. I did not copy and paste the actual code I thought I would try to boil it down to its most simple form. my string is not "major.minor.build.revision" My string would literally look like this 
    $mystring = "gouda.usa.texas.dallas";

    [download]
    I am using Config::Inifiles in fictitious dosumpton and validation would take there because if Section and Parameter are not in the config file then I will add default data or kill the job based on other parameters passed to dosumpton. dosumpton takes many more parameters than shown. I just wanted to know what corner cases the algorithm would miss. As a blue collar or more appropriately shadetree developer the algorithm is where I tend to have more short commings.
[reply]
[d/l]

      If you are saying you are passing a huge list (more than 3) of position dependent parameters to a sub then you are buying yourself trouble right from the get go. Instead load up a hash with the parameters as you discover them then pass the hash into the sub. That gives you the equivalent of named parameters. Consider:

      use 5.010;
 ...

my %params;
my @parts = split '.', $mystring;
$params{cheese} = $parts[0];
$params{country} = $parts[1];
$params{state} = $parts[2];
$params{town} = $parts[3];

doSumpton(%params);

 ...

sub doSumpton {
    my %params = @_;

    $params{cheese} //= '-- bad cheese --'; # Provide cheesy default
    print "Cheese: $params{cheese}\n";
}

      [download]

      Using a hash slice the assignments to the hash can be done as:

      @params{qw(cheese country state town)} = @parts;

      [download]
      True laziness is hard work
[reply]
[d/l]
[select]

      I can garantuee you that nobody here thought that the string was "major.minor.build.revision". Not because I can mind read but because your script is really not that complicated and none of the answers to your post seems to assume a fixed string.

      You might not be aware that in perl the following is true:

      @a=("one","two");
dosumption(@a);

# is the same as

@a a=("one","two",undef);
dosumption(@a);

# is the same as

dosumption("one","two");

      [download]

      And because of that the solutions offered to you are really equivalent to your script (with the exception of what happens to "" and "0" both of which result to false without being undef) but mich simpler and less error prone

[reply]
[d/l]

Back to Seekers of Perl Wisdom