Coupon script wtf ...
-
Not to name any names, oops, but there is a very popular coupon script out there that a recent client of mine want to be tweaked a little to suite thier needs. Figuring that she paid $129 for the software, and it was widely used, figured it would be "fairly" easy to follow along the code and insert her fix. To my surprise, this this the worst code imaginable, of the likes I have never quite seen before, except maybe when I was 5 or 6 writing my own code... I'll start with the HTML and move into the PHP ...
First, there are no <html> or <head> tags to start the page, it has a <title> tag but its just sitting there in space .. I dont know if the owner of the site took these out, but I can't imagine she had, as there were also no ending tags or even a <body> tag. The site is designed on a table layout, where the common practice of putting the left side bar in the header file, and the left ads bar is in the footer file, somehow makes sense. Where there are tags, its a mix of <TR> <Tr> and <tr> type code. The main index page takes in a parameter of $page and the index page goes through a series of ...
if($page=='main'){ require('main.php');} which I guess is "ok" but, then it starts doing ..
if($page=='something2'){ require('something.php'); }
if($page=='something2a'){ require('something.php'); }
if($page=='something2b'){ require('something.php'); }
if($page=='something2'c){ require('something.php'); }
if($page=='something3'){ require('something.php'); }
if($page=='something4'){ require('something.php'); } etc.. till something8
No, else if's or strpos (indexOf) or anything like that, that might help the code be more readable (and improve performance) ... But then again, performance didn't seem to be a critical issue.
Take the search functionality for example, there are about 8 criteria pertaining to a coupon search. I assume we all know the "proper", or at least semi decent way of doing this, but for what follows, I have no goggles strong enough to pretect my eyes.
pseudo code:
foreach criteria {
build an array of like statements such as '$criteria like '%$value%'
foreach statement in the array {
Open a database connection
select * from coupons where $like_statement
grab only the ID field and put it into an array
close the database connection
}
foreach ID found {
Open a database connection
select * from coupons where $id
get all the fields
add to array
close database connection
}
echo <table>
foreach row {
i=3; while ($rows) {
if(i==3) { i ==2 }
if(i==2) { echo <tr> }
some row output junk ..
i--; // something like that .. no use of %
}
}
}
If its not obvious, for each criteria in the search it opens a connection to the database, grabs all the fields but only records the id, then closes the connection, for each criteria, then opens a database connection again for each row that was already found ... I dare to see what would happen if one of these sites actually got some decent traffic, or a large number of coupons in its database. The same type of structure was also copied and pasted for its business search.
the previous version of this software had SQL Injection written all over it, and it doesn't seem like they did much of anything to take the attacks very seriously, the strings are still thrown inline but at least are now escaped, they could be bound though.
For $129 a pop, and its huge popularity, this code is a joke and a huge wtf line after line.
-
Seems like a good opportunity to write a better one (which would take what? a weekend?) and sell it for $128.
-
Shhh lol ....
-
@jminkler said:
For $129 a pop, and its huge popularity, this code is a joke and a huge wtf line after line.
Like any good comedy act, you gotta pay to see it. $129 isn't so bad if you look at it that way anyhow.
Then again, I've seen government funded projects (both state and US federal) worse that were costing taxpayers millions.
