• (nodebb)

    There's a deeper WTF though, that highlights a common anti-pattern that I've seen in many places: we're tying code logic to values in database fields. The code fields can't change in the database. Adding a new row to the recordTypes table won't handle the special case of rendering that new row- you need to roll out code changes to support data changes.

    Well, duh. You're handling a new type of record, so you need new code to handle it.

    What this type of problem indicates is that the database structure is incorrect, in the sense that the content should indicate the record type. Or maybe there's a single table with two (for large values of two) types of record in it, which is also a major no-no.

    But when we add a new type of information to the database, we will inevitably have to add new code to handle it.

    (In the context of the article, I'd say that the biggest immediate problem is that once you're down this rat-hole, you need to modify these switch()es (whatever the language calls them) everywhere. It's almost guaranteed that some day, someone making such a change will overlook one of the switch()es, and manage to find a hole in the automated tests, and ship stuff that breaks at runtime.)

  • Industrial Automation Engineer (unregistered)

    Pascal, 2002, starting development. 'Nuff said.

  • (nodebb)

    Oh, and I'd guess that the original is not in Pascal, since Pascal case statements don't require break; to separate different cases. There is no implicit fall-through in Pascal. Handling multiple values of the selector with a single block of code requires a comma-separated list of values:

       case somevalue of
      1: handle_case_1(...);
      2, 3, 4: handle_cases_2_and_3_and_4(...);
   end;
  • Smithers (unregistered) in reply to Steve_The_Cynic

    Would that mean the break;s are breaking out of the inner loop instead then? Seems reasonable since the matching record has already been found.

    Of course, I don't actually know Pascal, so for all I know they could be jumping out of the case completely pointlessly. (In which case it is also possible that the dev. responsible for this doesn't know Pascal as well as they thought either...)

  • Sauron (unregistered)

    Take all the hard drives on which a copy of that codebase has ever been stored.

    Throw the hard drives inside of a volcano.

    With the molten metal, forge rings of power and give them the pointy-haired bosses.

    Then grab popcorn and watch the world burn!

  • Nick (unregistered)

    I joined a project that used this kind of anti-pattern. We had a database table with half a dozen “record types”, and a bunch of code that linked behaviours to the record types.

    Adding a new record type required first adding the entry in the database, and then writing new code to handle that record type. I was bemused… why bother with the DB table at all, since we’re going to have to make code changes every time… why not just use an Enumeration or similar?

    The answer came a few weeks into the newest client project… when client X said “Oh, we only want to support record types 1 & 2, forget the rest”. Voila, no code changes, we just deleted the extra entries from the DB table, and those record types were no longer available for use. They’re still supported by the code though, so if that client ever changes their minds, we just need to add those extra types back in, and again, no code changes needed.

    After some thought, I’ve decided I quite like this pattern, and it has more value than I originally assumed.

  • Barry Margolin (github) in reply to Steve_The_Cynic

    I think the break statements are to break out of the for loop once a matching record type is found, not case

  • Maia Everett (github) in reply to Steve_The_Cynic

    Ah yes, the for-if anti-pattern.

    https://devblogs.microsoft.com/oldnewthing/20111227-00/?p=8793

  • (nodebb) in reply to Sauron

    Ash nazg durbatulûk

    Ash nazg gimbatul

    Ash nazg thrakatulûk

    Ag burzum-ishi krimpatul

  • dpm (unregistered) in reply to Sauron

    Now, Sauron, we've talked about this in our previous sessions. You really need to learn new tools and new methodologies. Remember the mantra we practiced? "Not everything needs to be hit with a hammer, even if it looks like a nail."

  • (nodebb) in reply to Smithers

    Would that mean the break;s are breaking out of the inner loop instead then? Seems reasonable since the matching record has already been found.

    Last time I touched Pascal for anything significant was ... (thinks) ... (thinks some more) ... well, it was a long time ago, and the Turbo Pascal versions I used then didn't have break; at all, but a little digging shows that it's a C-like break; that breaks loops even used inside case ... of statements.

  • (nodebb) in reply to Steve_The_Cynic

    well, this is because c is nuts using 'break' to mean 'end of case'. you either have no fallthrough or you use an 'endcase' statement like bcpl.

  • (nodebb) in reply to Steve_The_Cynic

    well, this is because c is nuts using 'break' to mean 'end of case'. you either have no fallthrough or you use an 'endcase' statement like bcpl.

  • (nodebb)

    i did not post the previous comment twice >.<

  • (nodebb)

    It's a join. We've reimplemented a join on the client side

    Unfortunately I have seen this "DIY join" anti-pattern many times. Also in the other form: the super big "IN" clause, with a very (VERY) long list of keys to be found - which is not just inefficient, it trashes the cache and kills the performance of ... well, everything else that uses the database. Among those cases, the worst one: this anti-pattern was combined with a totally unnecessary (ab)use of the C# "yield" feature, so the queries were executed multiple times (facepalm!)

  • (nodebb) in reply to Steve_The_Cynic

    @Steve:

    Or maybe there's a single table with two (for large values of two) types of record in it

    Or a type and a sub-type. Seen a million times. Often... er... did it myself :( What would the "correct" solution be in this case? A table for each of the two types and/or sub-types? This could be done when the value of two is two, but may not be viable for large values of two. e.g. an Invoice that has Items, and there are many types of items ... if there are 10 types of Item, shall I create 10 tables, all with FKs to the Invoice table? And that would not change the problem: when adding the 11th type of Item, I should modify the application code anyway, if only to CRUD the new Item type. Or am I missing something?

  • Anon55 (unregistered) in reply to Nick
    Comment held for moderation.
  • (nodebb) in reply to StefanoZ

    Fair point, although I was thinking of a more WTFish case where the "types" require more different handling than what you're suggesting, such as the distinction between "hammers versus screwdrivers" (at least they are both hand tools) and "apples and screwdrivers" (there's not much useful in common but at least they are both physical objects) or "justice and hammers" (they don't even have "physical object" as a shared attribute). If a table's contents are like justice and hammers, there's a real problem, but if it's "screwdrivers and hammers", they do at least share the commonality of both being hand tools.

  • (nodebb)

    Is this one of those cases where you want to look at NoSql?

  • (nodebb) in reply to Steve_The_Cynic

    I like it! "Justice & Hammers" sounds like a spinoff from "Law & Order", and maybe there is a crossover episode with Doctor Who: "Justice & Screwdrivers". Jokes aside, now that you've said that, I'm afraid I'll start seeing "justice and hammers" anti-pattern everywhere (starting from my code, of course...)

  • LZ79LRU (unregistered) in reply to StefanoZ

    One thing that comes to mind is OOP inheritance. As in figure out which fields are the same for all types and make that a base table. Than have specializing tables with just the data you need for the individual types and link those together.

    So your child item table might look like this: Child ID (PK), Parent ID (shared fields), unique field 1, unique field 2...

    And of course, if several children have their own wider subsets of shared fields you can just add another table for those: Root ID (PK), field 1, field 2... Middle Child ID (PK), Parent ID (shared fields), unique field 1, unique field 2...

    Leaf type 1 ID (PK), Parent ID (middle child), unique field 1, unique field 2... Leaf type 2 ID (PK), Parent ID (middle child), unique field 1, unique field 2...

    Assuming your system isn't completely apples and oranges this should get you something that is easy to implement and extent both in the DB and code. The only downside is that you have to suffer through the performance penalty of joins. But that's a choice you have to make based off the amount of data in your system and the number or lack there of of shared fields.

  • (nodebb) in reply to LZ79LRU

    You're right. And I especially like the bottom line: almost everything in our job is a trade-off. Most of the time I leave ER design to more expert people, hoping that that'll get it right, but nobody is perfect. The problem is that we often have to make choices quickly, with incomplete knowledge. And this is more problematic in databases: if you pick the wrong choice, you may be stuck with it for years, databases are hard to refactor because of the DATA. One day I'll make a submission about a very bad case of crazy ER design, but it's still too soon...

  • Strahd Ivarius (unregistered) in reply to dpm
    Comment held for moderation.
  • Ankit Chaudhary (unregistered)
    Comment held for moderation.
  • Richard A. O'Keefe (unregistered)

    The first thing that hit my eye was the break statements. Pascal 'case' statements, unlike C 'switch' statements, don't need 'break'. And that's doubly good, because Pascal doesn't have 'break'. This is in some weird Pascal dialect and written by someone who was trained on C or something. Nobody thinking in Pascal would write those 'break' statements.

Leave a comment on “Join Us Again”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article