• (nodebb)

    Perl in 2011. Uh-oh.

  • (nodebb)

    So, the first query is only there to ensure those IDs actually exist in the table? Is it even possible that they don't?

  • Industrial Automation Engineer (unregistered)

    refactoring a small portion is better than no refactoring at all, even if it is only to add comments. i've been at it for years at my current job, and the codebase improves slowly, and I shudder to think how I found it when I accepted the job.

    my favorite saying: "once deleted, it will never return."

  • (nodebb)

    So what happens if it "dies" I wonder? There is no context provided as to how this code is used.

  • (nodebb)

    Perl in 2011. Uh-oh.

    I've interviewed candidates that are using perl in 2024. Some of them were almost as old as the perl version they were using.

  • (nodebb)

    Hey, @Remy, you missed an extra WTF - the id field of the database is a string that's populated, it seems, entirely by stringified numbers. (OK, we only have a limited selection of values, but they are all stringified numbers.)

  • Aitkiar (unregistered)

    The pattern of join as a loop in code is very common in my actual job. It has to do with IBM funny way of calculating the amount of money they will charge you each month. In the old times the project i'm working now was a cobol + db2 mainframe monstruosity. Because the db2 and the cobol code execute over the same big machine, there where no network latency, and looping over a result to launch queries cost a lot less money than making a join query and looping over the results.

    We have moved from cobol, via automatic tranlation to java, and now we have tons of code with that kind of behaviour But now the core run in different hardware than the db2 so instead of money, we now are paying for it in network time, because of the trip to the db in each iteration of a loop.

  • Jaloopa (unregistered) in reply to Steve_The_Cynic

    How else are you going to support IDs that might be GUIDs? This is future proof code

  • CBG (unregistered) in reply to Steve_The_Cynic

    From that code, I wouldn't assume that the database field is itself a string. Quoting integers in queries is just another mini-WTF I've seen from junior coders who know little-to-nothing about data types, and think all data has to be in quotes.

  • Sauron (unregistered)

    I still harbor aspirations of being a supervillain some day.

    What are you waiting for? Become a supervillain TODAY!

    It'd be one heck of a story. The dev who sees bad code day after day, until he turns crazy and embraces a path of darkness as a revenge against society. Countless horrors happen, until the supervillain kidnaps Batman's girlfriend to make her write a web application in BOBX. An epic fight ensues, during which the server room gets destroyed, and the supervillain gets captured. But at the end of the film, his prison cell is found empty, save for piece of paper with a handwritten code snippet from the prison's electronic locks' source code, showing an unmistakable SQL injection...

  • Alexander (unregistered)

    Good rule of defense programming: check return codes/results, and if you don't know how to solve/process failure - just die/stop/halt.

    Do not try to continue up to unpredictable results.

    So, this "do or die" is actually not a WTF, but small piece of actual good code in this pile of shit.

    And yes, I sometime use perl for quick scripting. It's actually good as scripting language - if you code fit in ~5 kilobytes.

  • (nodebb) in reply to Steve_The_Cynic

    Not necessarily, it's possible it's an integer and they're forcing the database to convert the values for the comparison, per their implicit conversion type priorities.

  • Alistair (unregistered) in reply to Mr. TA

    There should be a foreign key constraint from admin_routeurs.networkid to admin_networks.id. If this is missing, or not enforced, it will be possible for the IDs to be missing.

  • (nodebb) in reply to Alexander

    if you don't know how to solve/process failure

    . . . then you should not be coding. Let someone who knows what they are doing write the application.

  • Ms. Sql (unregistered) in reply to Steve_The_Cynic

    That's OK. Microsoft SQL Server lets you search with stringified numbers for numeric values. As a side effect, the automatic conversion prevents an index search.

  • (nodebb) in reply to Michele Mauro

    Yop, I mean I coded in Pascal in the 80s as well, still know what it does, but honestly wouldn't release a project made with it in the 2010s. So it's great if you know stuff that long has been surpassed by better solutions, I wonder at this point though how long this project was actually cooking... over two decades, because that was pretty much when CGI was still popular?

  • (nodebb) in reply to Steve_The_Cynic

    I worked with a client ones where (ORACLE) SQL script literals all had to be strings no matter the datatype. Luckily I was in the .net team and we used code first migrations, so we had not to deal with this. However that was the old EF6 with XML migration scripts, so that resulted in another bag of bees during branch merges.

  • (nodebb) in reply to Steve_The_Cynic

    If it's MySQL, it automatically coerces strings to numbers when necessary, and it 's pretty common to quote numbers in queries (perhaps thinking it's a way to prevent SQL injection, or just because they've gotten into the habit of quoting all literals). It does no harm, so it's a minor WTF.

  • Argle (unregistered) in reply to Sauron

    It'd be one heck of a story.

    I'll watch it!

  • dusoft (unregistered) in reply to Barry Margolin

    Indeed, not a big WTF at all.

  • Randal L. Schwartz (github)

    This code is also subject to bobby tables, having used a string interpolation rather than a placeholder. Perl has supported placeholders since 1995 via DBI.

  • (nodebb)

    Another WTF: The id values in the query for $sth are not in order. Absolutely not required, but another person reading it would appreciate the ordering. Here's two lists; you decide which is more "readable"

    8,13,14,16,22,26,27,23,40,39,33,31

    8,13,14,16,22,23,26,27,31,33,39,40

  • Álvaro González (github) in reply to Bananafish

    Bu if you sort by number you lose history!

  • Duke of New York (unregistered) in reply to Randal L. Schwartz

    It's not, since the ID values are constrained in this context, but you can be sure that the developer has written other code that is.

    And if they would use placeholders, preparing the statement on each loop iteration would be unnecessary.

    This just illustrates that a "safety culture" can never be a match for a Turing-complete programming language.

  • Officer Johnny Holzkopf (unregistered) in reply to Álvaro González

    It is also possible that in the future IDs could be added in the futuristic form of '8a', '8c', '8.7', '8-1', '8 new', '8 deputy' and '8 '. This solution might still work... or die.

  • (nodebb) in reply to Mr. TA

    And would it even matter if they didn't?

  • (nodebb)

    This code is also subject to bobby tables

    Well, since the string interpolated is the id field from the previous query, it's probably already sanitised.

    What I like but that nobody seems to have picked up is that they are using prepare/execute but without any placeholders. They are so close...

    Addendum 2024-10-03 07:14: That's a response to Randal L. Schwartz

  • Torgo (unregistered)

    I wrote DB-interfacing Perl code many years ago that purposefully did a query/loop/query instead of one SQL join expressly because the indexing on the DB was so bad for a specific situation that doing multiple queries and looping was faster than letting the DB do all the work. But somehow I doubt that's the case here, especially since that first query is entirely superfluous even if you really want to have nested loops.

  • Torgo (unregistered)

    I wrote DB-interfacing Perl code many years ago that purposefully did a query/loop/query instead of one SQL join expressly because the indexing on the DB was so bad for a specific situation that doing multiple queries and looping was faster than letting the DB do all the work. But somehow I doubt that's the case here, especially since that first query is entirely superfluous even if you really want to have nested loops.

Leave a comment on “Join or Die”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article