• (nodebb)

    " Again, I don't fault the underqualified developer, I fault the organization which put them in a position they were guaranteed to fail."

    And this may possibly be the single biggest cause of WTF... Although people failing to stand up for them selves may well be an even bigger problem. Here in the USA (and in most global situations I am aware of) employment at a given organization is not forced. People do have the right to say "That I will not (or can not) do."

  • 🤷 (unregistered) 
    IF EXISTS (SELECT * FROM commentTable1)
BEGIN
    DROP TABLE commentTable1
    CREATE TABLE commentTable1 LIKE commentTable2
    INSERT INTO commentTable1(Comment) VALUES ('FRIST!')
END
  • RLB (unregistered) in reply to TheCPUWizard

    Here in the USA (and in most global situations I am aware of) employment at a given organization is not forced. People do have the right to say "That I will not (or can not) do."

    Easier said than done when it's your daily bread you're walking away from. If you're a successful programmer with dozens of years of experience and an emergency fund good for five years, you can do that. If you're fresh out of college, fell into this job because there aren't any to be found in your chosen profession (because circumstances change), and you can't afford to move halfway across the country to the big city? Not so easy.

  • Vilx- (unregistered)

    Did you also notice that there is not only a global variable $link, but also a function parameter $link? I'm not even sure which one gets used...

  • Prime Mover (unregistered)

    It doesn't matter if you're a brand new graduate straight out of uni or a seasoned consultant with 30 years experience behind you, you really should not leave your staff alone with no supervision and just say "get on with it"

  • Prime Mover (unregistered) in reply to RLB

    More to the point here, if you're a fresh new graduate you don't have anything to compare it to. For all you know, this sort of "minimal interaction" model with your staff may be the norm.

    There was an interesting post on Ask A Manager a while back where some guy was questioning the fact that he was told he needed to ask his boss's permission to leave the company and move on to a new job. after further investigation, said manager deliberately employed only fresh graduates, in order that she be able to mould them into what she wanted. And it emerged that many of her managerial practices were egregiously unacceptable in the context of modern society.

  • I dunno LOL ¯\(°_o)/¯ (unregistered)

    testOper

    And this is how you know the guy still had a working shift key. Suddenly, camelCase!

    And about $size being a string? I'm familiar with stringly math, but this guy likes his stringly enums.

  • Robin (unregistered)

    Kind of irrelevant because, as we all agree, the badness of this code isn't primarily the fault of the developer concerned - but I'm surprised there's no comment on the fact that the conditionals are in fact assignments, so the parameter passed to the function is irrelevant, we're always going to execute that drop_table statement.

    So if, God forbid, this is in production, not only is the development process messed up beyond all hope of fixing, but there's clearly not even any cursory testing.

  • Shiwa (unregistered)

    Since they use "if ($size = 'EXIST')" instead of "if ($size == 'EXIST')", the whole elseif is dead code.

    Did they hire a building architect instead of a software one ?

  • Jay (unregistered)

    This looks like a piece of my C++ homework from over a decade ago(I know it's not C++, but the structure of the logic is similar). Mixed equality operators which would short-circuit the conditionals (as Shiwa pointed out). Variable names without defined types which don't do what you would expect of them ("$size" is a string containing the result message of a query instead of, you know, a "size"). All that's missing is me spending 2 hours trying to debug this monstrosity when the project is due by the end of class..

    Re-reading the code, I get what he did. The "$size" parameter is a flag for whether he wants to create a new table or overwrite an existing table. He overwrites existing tables to make sure that the copy is exact. $Table1 is the destination table and $table 2 is the source table (backwards in my opinion, but I'd be lying if i said hadn't seen this in other commercial functions before).

    Why is he executing raw SQL strings? There could be reasons such as "no access to the database to create custom objects like Stored procedures" or simply "no idea what kind of DB system this is gonna interact with", hence these queries would work on any SQL-supporting DB system. It's still not good though.

  • Jay (unregistered)

    This gets further complicated.

    It looks like the "$requestz#" variables are defined outside the code snippet. Knowing which of the values is populated could tell you which path your function took, but otherwise, it's junk.

    "$copy" is also an argument passed to the function. My guess is that it was used to test the drop and create logic without copying the table. I'm guessing it's just a string that holds his magic "error" keyword.

    This guy either didn't expect his code to work at all, or added this stuff later to debug it when it failed.

  • Scott (unregistered)

    This reminds me of a idiot consultant I worked with just recently. Given a project to "mask" data for restoring test dbs from production.

    After a month or so, was telling me what he came up with. Wrote a windows app (no, I don't know why) to perform this.

    He was all excited by figuring this out. It creates a copy of each table, copies the data from the real table into the new, applying the masks. Then, it dropped all the real tables FKs/indices/etc, then dropped the real table, then renamed the copy to the real table's name and recreated the FKs/indices/etc.

    Me: "Why not just generate an update statement?"

    Him: "Hurr durr, it runs in just over a minute, that's fine."

    Sigh. Needless to say, I got away from those stupid fuckers.

  • Is That Really A Crosswalk? (unregistered)

    $Size = small, medium, or large

  • tbo (unregistered) in reply to Vilx-

    I think it takes the function parameter and makes it available at the global scope? But I've never actually used it like this, so I don't really know...

  • seebs (unregistered)

    I suspect that $result is a global because the db_query command can update it, which is why you check for it containing "error" after running the query.

  • Kleyguerth (github) in reply to Vilx-

    According to someone at Stackoverflow, the global variable shadows the function parameter. If you want to use the function parameter you have to use func_get_args. https://stackoverflow.com/questions/28708897/php-global-and-parameter-of-the-same-name

  • Sole Purpose Of Visit (unregistered) in reply to Kleyguerth

    In which case I can only say, "All your base are belong to us."

    Once more, flawless victory for PHP's habit of taking two choices of semantics and picking the obviously wrong one.

  • (nodebb) in reply to Scott

    Given a project to "mask" data for restoring test dbs from production.

    People gotta stop doing this. There's no better way to both put your customer's data at risk while simultaneously making your test environment behave unpredictably. The one consistent truth about real life is that it's messy.

    Cultivate a fictitious client base that you can make real statements about, and that should behave predictably. Make a bunch of bots that live their made-up lifecycles. It's a heck of a lot more fun than restoring from production.

  • (nodebb) in reply to Jaime

    The messiness of real life is one of the main reasons why the content of the test db would be modelled on production; it will have developed the weird edge cases that you wouldn't have thought of synthesising yourself.

    But you wouldn't keep doing it unless and until you need to reflect significant changes; you'd restore the test db from a backup test db, not production.

  • Scott Christian Simmons (google)

    I'm trying to figure out which language this developer is presumed not to know. Good cases to be made for either PHP or SQL here ...

  • (nodebb) in reply to Watson

    it will have developed the weird edge cases that you wouldn't have thought of synthesising yourself

    True story: I used to work for a pharmaceutical distributor that specialized in generics. We had this feature called "prebook" where we put customers on a list to buy a product the first day it goes generic. The nature of the feature was that each product collected orders over the months before the release, and all orders were filled on the morning of the release.

    No production backup ever had both prebook orders and stock of any product because backups happened overnight and all the stock that was put in the first day was sold out before the nightly backup. Because the team insisted on building test databases by cleansing production data, this feature never got tested thoroughly, and was the buggiest part of the entire application.

    Reporting is also a challenge with "natural data" test environments because you never really know what the report is supposed to show until you do a bunch of research.

    My experience has always shown that building test environments from prod helps with the simple cases, but actually makes the edge cases harder to test. It also makes improvement harder because prod goes whatever way it wants to. Every time you try to seed it with specific cases, your customers go back to their old boring ways.

  • MiserableOldGit (unregistered) in reply to Watson

    If the subtlety of any messy edge cases make it through anonymisation , you're probably not doing that right.

    It was made crystal clear to the team I was on 20 years ago that we mustn't be generating the test data this way, not least because of the ex-filtration risk of un-scrambled back up data lying around somewhere. That is, we were told to stop doing it, I do wonder how many years it was before they actually did!

  • MiserableOldGit (unregistered) in reply to Jaime
    My experience has always shown that building test environments from prod helps with the simple cases, but actually makes the edge cases harder to test. It also makes improvement harder because prod goes whatever way it wants to. Every time you try to seed it with specific cases, your customers go back to their old boring ways.

    Yes, this. Both customers and data staff in your organisation will be working round all sorts of known bugs anyway (not using hyphens and apostrophes in surnames, using a fake postcode in addresses without one, counting to 3 before pressing save). The "backup data" method won't get you the crappy process race conditions that happen (and also may very be being ignored as well) like orders somehow being shipped before being picked. And if you are relying on a customer called Bobby Drop Tables lurking in your database to find your newly baked injection vuln, you've got bigger problems!

  • jeremy (unregistered)

    Copytables(prod,backup,....); 'Oh noes prod is down. Abort abort abort ctrl C Copytables(backup,prod,...);

  • Some Ed (unregistered) in reply to Sole Purpose Of Visit

    Two?

    I've seen languages that would take a function definition with one of the parameters being immediately explicitly defined as a global in many different ways. If we expand the scope to all languages where this could conceptually happen, rather than limiting ourselves to ones that declare functions with the keyword function and do not define a type for them, declare globals with the keyword 'global', and so forth, then I've seen it done in the following 8 ways:

    1. Whatever is passed to the function is assigned to the global. This is great for routines that are designed to set global variables, you can just use this feature, so the body of the function is just the global declaration.

    2. The global variable is available in the rest of the script. If you want to access the parameter, you need to get it by inspecting the subroutine's call stack.

    3. The global variable is available to the rest of the script. The function parameter is not available via any means, as it's been released from memory.

    4. The function parameter is available to the rest of the script. The global variable is not.

    5. Reading the variable gives the function parameter, but assigning to it sets the global variable.

    6. The behavior is undefined by the language and varies by the implementation.

    7. This is a compile time error.

    8. This is a runtime error. What is this compile time you speak of?

    In addition to these, I could also imagine:

    1. Reading the variable gives the global variable, but writing to it updates the parameter. If it was call by value, this is basically a no-op.

    2. Reading the variable gives the global variable, but writing to it causes an abnormal exit.

    3. I knew a guy back in the day who was working on making a C compiler which would insert random op codes whenever it encountered anything that he knew was defined by the C standard to have undefined behavior. I doubt he succeeded, but it's conceptually possible.

Leave a comment on “Drop into the Deep End”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article