- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
history matters (and is not disclosed)...there can be significant differences between loops and "unrolled" implementations. These days, usually the compiler/runtime is smart enough, but it has not always been so.
queries in the database are quite common (hint, how do you think all of PowerBI works for example or SQL Server Reporting Services, or even regular Stored Procedures or so many other main stream items where people don't give it a thought.
Admin
"The status information is represented as a string of four integers" - why is a string of four integers not represented as a string? Is this not enterprise-leveraged? Oh well... string.missedOpportunity(value).equals(byteArray lpnszByteArray, x+1); // TODO: Frist boolean must be TRUE, FALSE, FILE_NOT_FOUND
Admin
@TheCPUWizard: Granted that those tools store queries in a DB. The difference is how they get created and edited and who does so. Big difference between Jane in Accounts Receivable can fiddle with the PowerBI UI to build a query vs. Bob in Accounts Payable can roll his own SQL and in fact alter queries (and
UPDATES
?!?) that the business depends on.Which are both quite different from Chris the DBA can edit stored procedures after the change board reviews them and the changed versions have been tested in the QA instances.
Admin
The
status
field isn'tfinal
and is initiallynull
, so there's some other call needed to set it;equals
will fail with aNullPointerException
otherwise. In other words, mutable global state! Yay!Admin
I haven't touched Java in a while (not that I've touched it a lot in the first place), but maybe something like that would work?
Admin
I actually don't understand how this works at all. Why are we testing the values of the array
status
when the array that actually contains values isnormal
?Admin
I we don't know enough about this system to know that this is being done wrong. The actual problem that's right in front of us is that queries aren't being properly parameterized. This is a last-century era mistake.
Stored procedures would have given the exact same level of "the query can be modified independently of the site", while still allowing proper parameterization, and the ability to introduce real database libraries that can make code that calls the database more reliable (type checking, nullability, special characters, etc..). I know it's PHP, but it's still possible to have some discipline.
Admin
Probably Remy snipped more code than necessary, so we don't see
status=normal
assignation somewhere.Admin
About the PHP code:
From what's written, this was made in a version of PHP before PHP learned about parameterization. The extension used has been removed from PHP since PHP7, and looking at the docs, there is no prepare line. All parameterization is basic string editing. If the PHP on that system is being kept up to date, they're using a shim to add the functions in the article back into PHP. Any code written now in PHP will either use PDO with the MySQL driver, or the MySQLi functions (note the i at the end), which are object-based I believe (and by the time I learned about those functions, I was already using PDO, as my job had me dealing with non-MySQL databases with PHP, so why learn multiple APIs when I can just learn one and use it with the databases I need to interact with?)
Admin
I once wrote an application that stored queries in the DB. It was like a bookstore with categories, and I wanted to be able to assign categories dynamically based on properties of the item. I thought I was quite clever.
But this table wasn't exposed to end users, a programmer like me updated it by hand.
But I probably didn't really need all this generality. In practice there were just three types of dynamic categories: featured authors, recent releases, and everything. I could have created a featured authors table that I join with, and then hard-coded the recent and all categories.
But I've always liked data-driven designs.
Admin
First off, let's look at that Java code. Everything is static! Is this meant as a singleton? In particular I'm looking at the status array and the equals function. Can the "normal" array ever change? If not, shouldn't it be final?
equals is normally an instance function, not static (unless two objects are being passed in). The passed in array should be checked for null (so should the status array). The length of array should be checked against the length of status. Comparing entries in array and status can be simplified into a loop which exits with the first mismatch.
As there are only four entries in the status array, either make public constants indicating what each index means OR break that array into one field per index. Doing this will depend on context which the snippit doesn't provide.
To be a stickler, php executes on the server, not the front end.
Others have mentioned that the mysqli package was introduced in php 5, with the mysql package completely removed shortly thereafter. This is old code. Prepared statements were introduced in the mysqli package. Lots of potential for injections the way it currently is.
Admin
As written, no, because it compares index 1 three times, and completely ignores indexes 2 and 3.
Admin
I suspect (okay, wildly hope) that the snipped code in the class includes a section that assigns values to the status array.
Admin
I'm afraid that would not work.
status[0] =3 and array[0] = 6 would indicate that they are equal (3 & 6 == 2). You need to use the Ex-or operator and compare to 0. For example:
public static boolean equals(byte[] array){ return 0 == (status[0] ^ array[0]) | (status[1] ^ array[1]) | (status[2] ^ array[2]) | (status[3] ^ array[3]); }
It is still a WTF though. Just do null checks, length checks (if appropriate) and loop the bytes. Simple is best sometimes.
Admin
Because it is not a sequence of printable characters.
Admin
Ah, right, I was too quick to copy & paste. I'm giving the wrong example here, making WTFs myself in the comments xD