Re: Re: Rogue 'if - elseif' code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, May 23, 2009 at 4:36 PM, Andre Dubuc <aajdubuc@xxxxxxxxxxx> wrote:
> On May 23, 2009, you wrote:
>> On Fri, May 22, 2009 at 2:50 PM, Nathan Rixham <nrixham@xxxxxxxxx> wrote:
>> > Andre Dubuc wrote:
>> >> Hi,
>> >>
>> >> I'm having problems with a chunk of 'rogue' code that does not perform
>> >> as expected (it does not pass the expected date, but an empty value).
>> >> Most of the time, it works - so I'm wondering whether it might be a
>> >> browser issue. (The latest failure occurred with Firfeox 3.0 browser on
>> >> an NT 5.1 OS.) The code is stored on an Unix server, running PHP 5.x.
>> >> I've isolated it down to multiple if - elseif statements that check long
>> >> dates for completeness, "day" "month" and "year". The code then applies
>> >> it results printing one of following: the full date - 22 May 2009,
>> >> month-year - May 2009, or just the year, 2009. My question is whether
>> >> the ORDER of checking is important, i.e, whether checking for the null
>> >> sets BEFORE full sets or AFTER.
>> >> I've added debugging code to be able to get the raw POST values so I can
>> >> manually enter it into the db, but I would really appreciate some help
>> >> here. The code is old -- I wrote it seven years ago, and had worked well
>> >> until I modified it, but I no longer have the original working code.
>> >> [The first conditional line of the code checks whether the user has
>> >> entered the birth date, then it checks for the death date. The $_SESSION
>> >> stuff is debugging code.]
>> >>
>> >>
>> >> <?php
>> >>
>> >>
>> >>        $yd = $_POST['death'];
>> >>        $yb =  $_POST['birth'];
>> >>
>> >>
>> >>        if ($_POST['bday'] == "Day" && $_POST['bmonth'] == "Month" &&
>> >> $_POST['birth'] == "Year") {
>> >>
>> >>
>> >>                if ($_POST['dday'] != "Day" && $_POST['dmonth'] !=
>> >> "Month" && $_POST['death'] != "Year") {
>> >>
>> >>                        $_POST['rdod'] = ("{$_POST['dday']}
>> >> {$_POST['dmonth']} {$_POST['death']}");
>> >>                        $_SESSION['ALL1rdod'] = $_POST['rdod'];
>> >>
>> >>                }
>> >>
>> >>
>> >>                elseif ($_POST['dday'] == "Day" && $_POST['dmonth'] ==
>> >> "Month" && $_POST['death'] != "Year") {
>> >>
>> >>                        $_POST['rdod'] = $_POST['death'];
>> >>                        $_SESSION['YEAR1rdod'] = $_POST['rdod'];
>> >>
>> >>                }
>> >>
>> >>
>> >>                elseif ($_POST['dday'] == "Day" && $_POST['dmonth'] !=
>> >> "Month" && $_POST['death'] != "Year") {
>> >>
>> >>                        $_POST['rdod'] = ("{$_POST['dmonth']}
>> >> {$_POST['death']}");
>> >>                        $_SESSION['MONTHYEAR1rdod'] = $_POST['rdod'];
>> >>
>> >>                }
>> >>
>> >>
>> >>                $_SESSION['Sdebugdod1'] = "{$_POST['dday']}
>> >> {$_POST['dmonth']} {$_POST['death']}";
>> >>
>> >>        }
>> >>
>> >>
>> >>                else {
>> >>
>> >>
>> >>                        if ($yd > $yb) {
>> >>
>> >>
>> >>                                if ($_POST['bday'] != "Day" &&
>> >> $_POST['bmonth'] != "Month" && $_POST['birth'] != "Year") {
>> >>
>> >>                                        $_POST['rdob'] =
>> >> ("{$_POST['bday']} {$_POST['bmonth']} {$_POST['birth']}");
>> >>
>> >>                                }
>> >>
>> >>
>> >>                                elseif ($_POST['bday'] == "Day" &&
>> >> $_POST['bmonth'] == "Month" && $_POST['birth'] != "Year") {
>> >>
>> >>                                        $_POST['rdob'] = $_POST['birth'];
>> >>
>> >>                                }
>> >>
>> >>
>> >>                                elseif ($_POST['bday'] == "Day" &&
>> >> $_POST['bmonth'] != "Month" && $_POST['birth'] != "Year") {
>> >>
>> >>                                        $_POST['rdob'] =
>> >> ("{$_POST['bmonth']} {$_POST['birth']}");
>> >>
>> >>                                }
>> >>
>> >>
>> >>
>> >>                                if ($_POST['dday'] != "Day" &&
>> >> $_POST['dmonth'] != "Month" && $_POST['death'] != "Year") {
>> >>
>> >>                                        $_POST['rdod'] =
>> >> ("{$_POST['dday']} {$_POST['dmonth']} {$_POST['death']}");
>> >>                                        $_SESSION['ALL2rdod'] =
>> >> $_POST['rdod'];
>> >>
>> >>                                }
>> >>
>> >>
>> >>                                elseif ($_POST['dday'] == "Day" &&
>> >> $_POST['dmonth'] == "Month" && $_POST['death'] != "Year") {
>> >>
>> >>                                        $_POST['rdod'] = $_POST['death'];
>> >>                                        $_SESSION['YEAR2rdod'] =
>> >> $_POST['rdod'];
>> >>
>> >>                                }
>> >>
>> >>
>> >>                                elseif ($_POST['dday'] == "Day" &&
>> >> $_POST['dmonth'] != "Month" && $_POST['death'] != "Year") {
>> >>
>> >>                                        $_POST['rdod'] =
>> >> ("{$_POST['dmonth']} {$_POST['death']}");
>> >>                                        $_SESSION['MONTHYEAR2rdod'] =
>> >> $_POST['rdod'];
>> >>
>> >>                                }
>> >>
>> >>
>> >>                                $_SESSION['Sdebugdod2'] =
>> >> "{$_POST['dday']} {$_POST['dmonth']} {$_POST['death']}";
>> >>
>> >>
>> >>
>> >>                        }
>> >>
>> >>
>> >> ?>
>> >>
>> >> Any help or pointers would be greatly appreciated!
>> >>
>> >> Tia,
>> >> Andre
>> >
>> > makes no sense at all to me without some decent variable names "rdod"? -
>> > would also need the form to be honest, and a description of the
>> > functionality needed.
>> >
>> > one thing i can say is that you have a closing bracket missing at the end
>> > and your $_SESSION['Sdebugdod2'] appears to be in the wrong place
>> > (to solve both these try sticking a } before $_SESSION['Sdebugdod2']
>> >
>> > really though, change the names in the form to something more descriptive
>> >
>> > --
>> > PHP General Mailing List (http://www.php.net/)
>> > To unsubscribe, visit: http://www.php.net/unsub.php
>>
>> Yes, additional context would be great.  Also, it looks like Andre is
>> storing all his variables in $_POST, making it even harder to tell
>> what is from the HTML form and what is a programer's variable.  Andre,
>> IF I'm interpreting your explanation and code right, you are accepting
>> an entire date in a single field and it could be complete or partial.
>> If this is the case, you could much more easily validate the field
>> with a few regex patterns.  If you still need to work with the
>> individual components, either have separate date component fields in
>> your form, or use preg_split($regex,$date,PREG_SPLIT_DELIM_CAPTURE) to
>> parse the single input field for the date components.  For preg_split,
>> make your regex match valid date components and combined with the
>> DELIM_CAPTURE argument it will return an array of the date components.
>>  From there, do any further validation you need on the individual
>> components.  This would significantly shorten your code and make it
>> easier to read another 7 years from now.  And for goodness sake, I
>> hope you comment your code these days.
>
>
> Nope, I'm not 'storing' all my variables in $_POST -- this is the first
> processing from an input page where the input variables are passed as $_POST
> variables (since they have not been evaluated/sanitized yet).
>
> The input for the date of birth/death fields come from drop-down three section
> HTML selects - the resulting data may output a complete or partial date.
>
> I need to work with individual components -- regex would be overkill here and
> result in even longer code. The code is quite simple once you understand that
> the Date of Birth is optional, but the Date of Death is mandatory. The prefix
> for Date of Birth 'b' as in 'bday' bmonth' 'byear', the prefix for Date of
> Daeth is 'd'.
>
> As far as readability, I have no probs wih it - and since I shall be the only
> one ever working with it, I'm sorry it was a problem for you. Actually, this
> part of the code for the page I felt didn't need any commenting: it was
> self-explanatory (at least it is to me. Later code is very heavily commented
> because it becomes very complex with multiple conditionals.).
>
> However, the main question that I had asked, and is still unanswered: is the
> ORDER of condtionals significant? Should one check for NULL first or last, or
> check for the complete dat then move through various combos until NULL?
>
> I must say I am surprised by the tone of first responses to my question. I
> guess the 'temper of the times'  has changed somewhat since I was last on.
> Pity.
>
> Thanks for replying.
> Andre
>
> Btw, I've gone off list, so don't feel obligated to reply.
>
>

1 - You're not the only one "working on it" once you post it to a
mailing list asking for help.

2 - $_POST['rdod'] and $_POST['rdob'] sure look like they're being
used as intermediate variables.

3 - That little detail of 3 select fields made all the difference in
understanding what you're trying to accomplish in the code.  A great
place to note such things is in... a comment...

Anyway, my answer to your base question is... I always check for null
first.  If there is no data, no point in doing anything else on it.

Aside from that, I agree regex isn't right, now that I understand the
application better.  Regardless, I think this section of code still
seems way too complicated, redundant, and convoluted for what you're
trying to accomplish.  With a chain of if-elseif statements like that
its terribly inefficient and the redundant conditionals makes problems
hard to spot.  Please consider the following logic:

$rdob = '';

if (isset($_POST['birth']) && $_POST['birth'] != 'Year') {
    $rdob = $_POST['birth'];
    if (isset($_POST['bmonth']) && $_POST['bmonth'] != 'Month') {
        $rdob = $_POST['bmonth'] . ' ' . $rdob;
        if (isset($_POST['bday']) && $_POST['bday'] != 'Day') {
            $rdob .= $_POST['bday'] . ' ' . $rdob;
        }
    }
}

Each field is only processed once while building the $rdob string.
When a required component doesn't check out it stops.  Now if you need
to process all 3 components, you might want to do the test separately
and store the boolean result for use later.  That way you can reuse
the test results and only have to edit the logic in one place.  As far
as those multiple session vars, I have no idea how you plan to use
them, but there has got to be a better way.  Modify this logic to fit
your needs if you wish.

Glancing at it, I don't notice where the problem you're experiencing
might be in the section of code you provided.  It might be in a
completely different place for all we know.  Its probably a simple
typo that's hard to find because of how it was written.  Anyway, thats
all I have to offer.  Good luck.

-- 
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php



[Index of Archives]     [PHP Home]     [Apache Users]     [PHP on Windows]     [Kernel Newbies]     [PHP Install]     [PHP Classes]     [Pear]     [Postgresql]     [Postgresql PHP]     [PHP on Windows]     [PHP Database Programming]     [PHP SOAP]

  Powered by Linux