http://qs1969.pair.com?node_id=96146

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

Ok I would like to make my "if" look better. I'm getting data from a web form using CGI. The data I'm checking is the Ship-to address, they don't have to fill in these fields but if they do that have to fill in the required ones. I have come up with this so far.
if ( ( $query->param("Shipping_address") || $query->param("shipCity") || $query->param("shipState") || $query->param("shipcountry") || $query->param("shipZip") || $query->param("shipdayph") ) && ( !$query->param("Shipping_address") || !$query->param("shipCity") || !$query->param("shipState") || !$query->param("shipcountry") || !$query->param("shipZip") || !$query->param("shipdayph") ) ) { failure(); }

------
PT - Perl Tanks %100 Perl programming game
The Price of Freedom is Eternal Vigilance

Replies are listed 'Best First'.
Re: Nicer If...
by suaveant (Parson) on Jul 12, 2001 at 23:09 UTC
    Create a ship_fields array with all the shipping fields then do something like this....
    my($has,$missing); $query->param($_) ? $has++ : $missing++ for @ship_fields; failure() if($has && $missing);
    it could be golfed, but this is just proof of concept type stuff :) easily adapted. Could even have it track which are empty...

                    - Ant

      I like your idea. It's well suited for the example situation. But...
      It doesn't scale.
      If you have a long list of fields to test, maybe an unknown number of fields, it makes sence to stop checking in the moment you find a mismatch.

      This sub (that I use in a project) is both generic and scales well. It's not as Perlish beautiful as some examples, but it's quick. As you will see, it's part of a module.

      ######################################################## # Return true if all parameters are either defined or # undefined. Return false if there is a mix. ######################################################## sub defined_all_or_none { #my $self=shift; # This is a module, remember? shift; @_||(return 1); if(defined($_[0])) { for(@_) { unless(defined) { return 0; } } } else { for(@_) { if(defined) { return 0; } } } 1; }

      Update:
      Seeing it on the screen made me see clearer. :-)
      The statement:

      my $self=shift; # This is a module, remember?
      should be reduced to:
      shift;
      since $self is redundant. Sorry about that.

      f--k the world!!!!
      /dev/world has reached maximal mount count, check forced.

      Edit Masem 2001-07-13 - Code Tags

        You need code tags so your [0] shows up...

        Actually I like runrig's solution better than mine, it still goes through all the options, but it can be very simply tweaked to return which fields were empty, which is quite useful in this case. Your way will work, but since the calls for the data are retrieved via methods, and not a data structure, your way still has to call them all to be successful. You could do it otherwise, but then you would lose genericity. For something like this I prefer either a few lines of code in the prog, or a sub like yours that you pass params and a list of fields, but as I said, its not so generic any more....

                        - Ant

Re: Nicer If...
by runrig (Abbot) on Jul 12, 2001 at 23:12 UTC
    One idea:
    my @req_fields = qw( Shipping_address ... shipdayph ); my @filled = grep { $query->param($_) } @req_fields; failure if @req_fields && @filled
    Update: I like suaveant's idea, though I think the conditional would be 'if $has == @ship_fields or $missing == @ship_fields'

    Another update: Took suaveant's suggestion(s), thought I caught that first one :)

    Ok, I get it now, duh..

      similar to mine, but probably a little better, if you did !$query->param($_) it would return a list of empty ones, tho, which would allow you to throw a useful error to the user, the check even still works, but I'd just check to see if $#filled == $#req_fields || !@filled, instead of hardcoding a number

                      - Ant

      actually, since he wants them all, if one of them is not equal to zero, then you fail... the check is right, just tricky

                      - Ant