• Weretaco (unregistered)

    There are actually two bool values that are different.

  • Tom (unregistered)

    I actually found 2 parameter values that differ...

  • (nodebb)

    So, @Remy, the WTF of the name of the function that contains this admittedly-bogus if() thing isn't worth mentioning? MyAction ??? Really?

  • TH (unregistered)

    Of course, it would be easier to spot the difference if they had stayed on one line.

    Not that this isn't still an abomination, of course...

  • Rob (unregistered)

    There are two different boolean values. If you copy just the method calls, line 9, value 2 is true in the if block and false in the else block, and line 12, value 5 is false in the if block and true in the else block. (There's also an extra trailing space in the else block.)

    Of course they should still be using customer.someProperty and !customer.someProperty.

  • Scragar (unregistered)

    This kind of arguments list is why I personally think named arguments should be mandatory for anything unclear.

    Also sensible defaults that lets you just do:

        _customerService.UpsertSomething(
        id: customer.Id, 
        code: customer.Code,
        name: customer.Name, 
        address: [
            customer.Address1, 
            customer.Address2,
            customer.Zip,
            customer.City, 
            customer.Country
         ],
         someproperty: customer.someProperty,
         template: customer.TemplateId,
         misc: MiscEnum.Standard
    );

    It's so much easier to follow.

  • Vera (unregistered)

    Found the difference! It's the middle true/false on the third-to-last line! ...My reward is apparently some mild frustration and a sense of relief that I'm not the poor bastard working in this codebase.

  • (nodebb)

    Actually, there are two differences: the 47th parameter changes from "true" to "false", while the 71st parameter changes from "false" to "true".

  • (nodebb) in reply to dpm

    And that's why they had to use an if clause! Sure, for the 47th parameter they could have used the customer field directly, but the 71st parameter is behaving the opposite so this is not possible any more. No chance, they had to use an if clause, no WTF in this case.

  • (nodebb) in reply to Melissa U

    I agree completely. Remember, if you have 85 parameters for your function, you've definitely forgotten one and should make it 86.

  • (nodebb) in reply to Melissa U

    the 71st parameter is behaving the opposite so this is not possible any more

    Yeah, because it's, like, totally impossible to create an expression that does a true<=>false inversion of a.

    Addendum 2025-09-10 10:14: of a boolean value...

  • no (unregistered) in reply to Steve_The_Cynic

    (false != customer.someProperty) ? false : true

    there, that should do it (:

  • Jonathan (unregistered) in reply to Steve_The_Cynic

    Woosh!

  • (nodebb) in reply to dpm

    I think you meant to say "if you have 85 parameters for your function, you should 86 the function"

  • (nodebb) in reply to cellocgw

    [applause]

  • Die Kuhe (kein roboter) (unregistered)

    I would say it's the place where the good old ?: (ternary) operator could be useful for some parameters.

    But "you other people" always hate the ?: ...

  • Rob (unregistered) in reply to Die Kuhe (kein roboter)

    A ternary operator is great if the result is not a boolean. Any ternary expression that results in a boolean can be rewritten as a combination of && and ||.

    And you should of course be very very very conservative about nesting ternary expressions.

  • (nodebb) in reply to Die Kuhe (kein roboter)

    I have no idea why you're focusing on a ternary operator when the function has fscking eighty-five parameters. Seriously, that's your priority? I'd ask how do you know you have the right spot, you could be changing the 48th instead of the 47th.

  • (nodebb)

    "fsck - check and repair a Linux filesystem"

  • (nodebb)

    TRWTF is that they didn't put braces around single multi-line statements after if/else.

  • Lurk (unregistered)

    At the point they started passing in multiple fields/properties of a customer instance someone /should/ have woken up to the fact that perhaps, just /perhaps/ the way to go was with a couple or three DTOs, starting with Customer, and beat the method signature into something sane. So for, my money, the WTF is abject failure to refactor.

  • erffrfez (unregistered)

    What does it mean to "upsert" something?

  • Duke of New York (unregistered)

    boolean blindness meets boolean diplopia

  • (nodebb) in reply to erffrfez

    https://letmegooglethat.com/?q=upsert

  • (nodebb)

    Funny, that one could be 100% code from the last professional project I was involved with. Business services were all static classes which results in endless argument lists and the custom "ORM" was just a messy nightmare of SQL string concatenate and didn't even implement the Dispose pattern correctly.

  • dkf (unregistered)

    85 parameters, and around that number of columns in the underlying table. Worse, someone thought this was a good idea.

    Also, lots of boolean columns. There are use cases for them (I've seen several) but they're pretty rare. The misdesign runs deep here...

  • Argle (unregistered)

    I watched "The Purge" last night. I'm game.

  • Duke of New York (unregistered) in reply to Balor64

    Still posting LMGT links in 2025? How utterly miserable you must be.

  • Tgape (unregistered) in reply to Rob

    Any ternary expression that results in a boolean can be rewritten as a combination of && and ||.

    condition ? varA : varB

    That's simple, right? Except varA is sometimes true and sometimes false.

  • erffrfez (unregistered) in reply to Balor64

    oh crap, it really is a word!

  • (nodebb)

    Normally there are five differences in these "spot the differences" pictures but I could only find two.

Leave a comment on “Upsert Yours”

Log In or post as a guest

Replying to comment #684184:

Log Out

« Return to Article