Skip to main content
Submitted by Xenoveritas on
Topics

OK, so this post is probably going to be mostly opaque to non-programmers, but that's fine. It's a rant. You don't have to understand it.

When I first inherited this web app, one of the first things I did was to move various display logic (namely, the URLs of icons) out of the database and into the display side. I did this in a somewhat "hackish" sort of way by just creating URLs named after the primary keys of the table. (A cleaner way might be to use CSS.)

Any way, time goes on and I move on to other things. Eventually I'm asked to come back and help clean some things up.

Someone's gone and moved display data back into the database. (Namely, a list of URLs to icons, along with some other display data.) Er, fine, I guess[1]. At least that means if a new instance gets added to the database it will display without issue, right?

Wrong. Because along with moving display data into the database, the same someone hardcoded the number of rows and their keys into the display logic!

So we've got display data tied to the database and the database tied to display logic. Changing either involves changing both.

The best part about the keys being hardcoded is that the way it's set up, it assumes that the database always returns the rows in primary key order. Which would be OK if the query to fetch them included "ORDER BY PK" but it doesn't.

Also amusing is the way he's doing this:

for (int i = 0; i < 3; i++) {
    out.writeln("<a href=" + (char)34 +
            "foo.jsp?bar=" + (i + 1) + (char)34 +
            ">" + getName(i+1) +
            "</a><br>" +
            "<br>");
}

Yes, that's right, the for-loop is 0-based, while the key is 1-based. Rather than for (int i = 1; i = 3; i++) it adds 1 to i whenever it's used.

Anyway - back to trying to clean this up.

[1] Actually, no, not fine. Because this means that fixing errors with the display involves editing the database. And, yes, this has come up. Someone needed to fix the display data and foolishly thought it could be done without database access. It changes a display fix from being a simple "here's the new CSS file" to "someone needs to run these UPDATE statements against the database." Which becomes "everyone working on this project, you need to run these UPDATE statements on your copy of the database." So, no, don't do this. Ever.