• frist (unregistered)

    Fritz!!

  • Hans (unregistered)

    No unicorns? I miss the highlight of my day!

  • Tim (unregistered)

    We actually have a word for this in German: we call it "Denglisch".

  • huppenzuppen (unregistered)

    It's called Denglish (https://en.wikipedia.org/wiki/Denglisch)

  • RLB (unregistered)

    This is neither a for-switch nor a WTF. This is an almost reasonable way to check data availability based on an unknown set of filters. Sure, it's ugly, but that's enterprise logic for you. The only thing that really bugs me is that it seems to check the more wildcarded filters before the more restricted ones, which seems backwards. Had the conditions been if (!(i & 1)) &c., I'd have been entirely unflabbergasted.

  • Ashlea (unregistered) in reply to RLB

    Your sed s/WTF1/WTF2/ is not a solution. Luckily enough, this piece of software is dying for several painfully slow years now.

  • LCrawford (unregistered)

    I'd say this is not remarkable, except for the awkward RELEASE_4 versioning issue. Now if it was 16 combination sequence checks unrolled, with either nested if()s or goto(), then it would surely qualify as a primo WTF.

  • I'm not a robot (unregistered) in reply to RLB

    Agreed. for-switch is when it would be simpler to just write the steps out without using a loop at all. Trying to do that with this code would make it worse, not better.

    (Not necessarily saying there's no better way to achieve the goal, maybe even one that happens not to use any loops, but that would require a much bigger rewrite than just unrolling the existing loop and removing unreachable branches.)

  • (nodebb)

    I've used this kind of loop to test all possible combinations of a series of true/false switches, and I definitely felt like a was TRWTF for not coming up with something better... But at least it worked, and it was not for production code. What really bugs me is this line: for (unsigned int i=1; i<16; i++) // Test all combinations Unless it's an anonymization problem, they are missing the "all flags false" combination...

  • (nodebb)

    Debugging that must be a joy.

    I don't see why. As long as you don't have to test prior releases. Wait...

  • Zenith (unregistered)

    For switch is helpful when you have a process that can be rolled back or is reentrant. Since many developers can't manage a repeatable function (works on arguments, not global state), no wonder this looks scary. I agree that debugging must be "a joy" but only because of that VERSION_4 check which has other implications.

  • Barry Margolin (google)

    This is basically a replacement for nested loops:

    for w in ("", m_strActualPromo) { for x in ("", m_BAR.m_TLC_FIELDS.OBJECTCODE) { for y in ("", TOKEN(strFoo, H_BAZCODE)) { for z in ("", TOKEN(strFoo, H_CHAIN)) { strKey = w + x + y + z if strKey != "" && m_lDistributionchannel.GetFirst(strKey) { m_BAR.m_TLC_FIELDS.DISTRIBUTIONCHANNEL="R"; break } } } } }

    Better would be to use a library function that simply generates all the combinations of a collection.

  • Sole Purpose of Visit (unregistered) in reply to RLB

    It's not "almost reasonable" at all. It's a Dunning-Kruger imbecile playing with his bits. What it implies is that the data-source if completely FUBAR, because, who on earth would index a data source with a four-part arbitrary key like that? It doesn't even have separators -- it just has single spaces denoting "absence of part," which is going to be tricky if a future requirement comes up whereby an embedded space is allowed in one of the four parts.

    It's ridiculously obtuse. What happens if the ordering is important in the business logic? (It almost certainly is.) Rewriting this would be a nightmare. Writing tests for it would be a nightmare.

    The versioning check is the icing on the cake. One would assume that there is a reason for the versioning. One would assume that that reason is related to the format of the four-part key. Chances are, the ordering changes, or the aforementioned non-separator problem surfaces, or ... just about anything, really. In which case you are probably better off with recursion, plain and simple. If, indeed, you have no control over the data source.

    The only saving grace is that this is in no way MFC, and it's only tangentially C++. There is no possible way to defend it.

  • Chris (unregistered)

    The only thing I don't like (besides the release version check) is the string replaces. Would it not be more efficient and easier to read if they just built the string each time (or if they used something like C#, used a StringBuilder)?

    Also, I would agree with RLB that surely they should be using all the information first, and going down to the combinations with less information. Then again, I'm not sure what's more likely to return a result first. If they wanted to do that though, instead of using !(i & 1), they could simply count down from 15 to 0, instead of 0 up to 15.

  • Olivier (unregistered) in reply to Chris

    "they could simply count down from 15 to 0, instead of 0 up to 15."

    Not quite.

    It should be 15, 14, 13, 11, 7, 12, 10, 9, 6, 5, 3, 8, 4, 2, 1, 0, that is numbers with 4 bits at 1, 3 bits at 1, 2 bits at 1, 1 bit at 1, no bit at 1.

  • (nodebb) in reply to Olivier

    It's not knowable from the information given what the priority should be. It might be, for instance, that a successful search with the specified promo code should take precedence over any search without the specified promo code. So then (assuming the others were of equal priority) you'd have 15 > (13, 11, 7) > (9, 5, 3) > 1 > 14 > (12, 10, 6) > (8, 4, 2) > 0.

  • Letchprecaun (unregistered) in reply to Zenith

    Remember code like this?

    Public ReadOnly Property IsNT() As Boolean Get Dim VerinfoEX as OSVERSIONINFOEX
    Dim Major as String = "" Dim PlatId as String = "" VerinfoEX.dwOSVersionInfoSize = Len(VerinfoEX) Dim blnRetVal As Boolean = GetVersionExX(VerinfoEX) If Not blnRetVal Then bVerEx = False Verinfo.dwOSVersionInfoSize = Len(Verinfo) blnRetVal = GetVersionEx(Verinfo) Major = Verinfo.dwMajorVersion.ToString.Trim
    PlatId = Verinfo.dwPlatformId.ToString.Trim
    End If Return (Major = 4 ) And (PlatId = 2) End Get End Property

    And people wonder why XP broke so much software....

  • (nodebb)

    Debugging that must be a joy.

    Just ... Yuckiness -- but what does the O in JOY stand for?

  • (nodebb) in reply to P. Wolff

    Obnoxious?

  • Altaree (unregistered)

    Are you sure they weren't just working on SAP? 6 letter German abbreviations for database column names. All text fields are space/zero padded to fixed widths.

  • (nodebb)

    if(IS_CONDITION("RELEASE_4"))

    While that is a trivial (and possibly poor) implementation, Feature flags are a powerful tool, and are gaining broad use. Properly done, it should NEVER be required to "Roll-back" a software deployment - just turn the feature off. Also, incremental rollout.

    On top of this, they can virtually eliminate "merge hell" as work can be done safely directly on "master" (or whatever you choose to call it), with the feature turned on for deployment only once the acceptance tests pass.

Leave a comment on “A Bit Masked”

Log In or post as a guest

Replying to comment #:

« Return to Article