IWETHEY v. 0.3.0 | TODO
1,095 registered users | 1 active user | 0 LpH | Statistics
Login | Create New User
IWETHEY Banner

Welcome to IWETHEY!

New validate this
I had to fix some old broken code, here's the failing part of the subroutine:
\n        my $invoices = $dbh->selectall_arrayref("\n            SELECT\n            *\n            FROM\n                cl_orders\n            WHERE\n                    cl_orders.cid=?",\n            { Slice => {} }, $sid) || die $dbh->errstr;\n\n        unless ($sid < 0 and $sid > -8 and $args{key} == 1) {\n            # -1 to -7 = test schools, so won't appear in cl_orders etc\n            return undef unless grep { $_ == $args{key} } map { ($_->{registration_num}) } @$invoices;\n        }\n

That has to be some of the worst code I have ever seen. "registration_num" isn't even a number, it's alphanumeric.

here's my version:
\n    my $invoices = $dbh->selectrow_array("  SELECT  1\n                                            FROM    cl_orders\n                                            WHERE   cid = ?\n                                            AND     registration_num = ?",\n                                            {}, $sid, $args{key});\n\n        # -1 to -8 = test schools, so won't appear in cl_orders etc\n    return undef unless($invoices || (($sid < 0) and ($sid > -8))); \n

I hope that's a little more clear.
Have fun,
Carl Forde
New Umm
"select 1"

"1"?

So this is checking for existence?


I really don't like multiple conditions in an 'unless'.
Something about the fact it is reversing the test (in my head) that make additional booleans within it confusing.

I'd test for the bad $sid to start off with and not bother with a database call if it is not going to be used.

And I've seen FAR worse code.
New ICLRPD (new thread)
Created as new thread #240536 titled [link|/forums/render/content/show?contentid=240536|ICLRPD]
-----------------------------------------
No new taxes.
--George H. W. Bush

We don't torture.
--George W. Bush
     validate this - (cforde) - (2)
         Umm - (broomberg) - (1)
             ICLRPD (new thread) - (Silverlock)

Just the facts, ma'am.
62 ms