• P (unregistered)

    Deep VB? More like Deep Blockly or Deep RPA, because the same kind of code is all over there too.

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

    Deep VB? More like Deep Hurting.

    But what I really want to know is if someone got Mexico to pay for this wall.

  • some guy (unregistered)

    Even from one application over, Resharper is screaming: "Invert if to reduce nesting"

  • my name is missing (unregistered)

    It's the End If of the world as we know it.

  • WTFGuy (unregistered)

    Certainly the code could be rewritten to be more performant. Although current VB.NET compilers are pretty good at hoisting subexpressions.

    The real fun here is what sort of convoluted business process generated a requirement like this. Stupid business logic will (almost) always produce stupid code.

    We all know we're looking at an anonymized excerpt of the true code; much more is missing than is shown.

    But as shown, the output of the loop is that LastReading will be set to the value of the Present field (or to zero) of the last wall-of-if's qualifying record in the retrieved record set. One wonders what value LastReading is initialized to above the loop. It's a good bet one could get a nice speed-up by iterating the record set in reverse order & quitting on the first wall-of-ifs match.

    Look at the bright side: At least some young dev hadn't written the same thing in LINQ. Sometimes "query comprehension" is really "query incomprehensible".

  • MiserableOldGit (unregistered)

    Can't be sure, but it looks like a load of business rules/business logic is hardcoded into that chunk of code. Other than that, it's kinda beautiful.

  • Steve (unregistered)

    So TRWFT is that they didn't use Continue For ? :)

  • JimJ (unregistered)

    It would be even better if there were Else somewhere in 8th level of nesting.

  • Steve (unregistered)

    I'm particularly fond of:

    If tempLocation = "CCCCCC" Or tempLocation = "CCCCCC" Then

    reminds me of the student who kept writing:

    A = A

    to make sure the assignment took.

  • Foo AKA Fooo (unregistered) in reply to some guy

    Now that would be a real WTF. Making every single condition harder to read (because it's inverted) just so coding style allows you to avoid indenting.

    Next week, we'll learn about the magic of the "and" operator.

  • sizer99 (google) in reply to Steve

    That's because when they print A it doesn't have the value they expect, which obviously means that they did properly set the correct value but it didn't stick. So better set it some more. That is real student thinking, I've seen it too.

  • sizer99 (google) in reply to Foo AKA Fooo

    The 'and' operator can't salvage this either. There is nothing that can make this readable and maintainable because the fundamental logic of the business rule is complete garbage.

  • Aspie Architect (unregistered)

    Years since I've seen a GOTO statement.

  • Foo AKA Fooo (unregistered) in reply to sizer99

    It would remove most of the indentation, which was the point of the comment I was replying to. No-one said anything about making it maintainable.

  • (nodebb)

    Hey! That code looks like my worst Oracle query execution plan ever.

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

    "Come From," surely?

    I've always wanted to use a language with Come From. Preferably one that adds pattern matching to the thing.

  • Gerry (unregistered) in reply to Foo AKA Fooo

    IIRC, classic VB doesn't support boolean short circuiting, so use and would mean that every condition was checked EVERY time. Except some sort of null reference exception would occur fairly quickly.

    basically, you can't use something like if (value <> null) and (value.field = "XXX"), as value.field would always be referenced.

  • Jaloopa (unregistered) in reply to Gerry

    VB.Net has AndAlso and OrElse for these situations. It's generally recommended to use them instead of And and Or unless you specifically don't want short circuiting

  • (nodebb) in reply to Gerry

    IIRC, classic VB doesn't support ...

    This is VB.Net. It uses features of the For construct that were introduced in version 1.1 of .Net. That means they have AndAlso.

  • (nodebb)

    It would not be such a problem if the indentation level would only be just 2 spaces instead of 4 (apart from the incorrectly indented first shown line, which I assume is indented 3 spaces), wouldn't it? More payload visible on the screen to bedazzle us!

  • (nodebb)

    "I don’t know what this code does"

    Well, it changes the value of LastMonth and LastReading. Or not. Whatever...

  • (nodebb)

    With a classical c or Pascal eye, you would have to declare tempLocation a lot earlier. And here, I would do the same thing anyway so you only need the assignment thirty doors down.

    Yes, you run the 'risk' of declaring it unnecessarily (but just the once), but you could also end up with the declaration statement being executed (putting some strain on memory management) a possibly infinite number (well, actually R, to be exact) of times.

  • just some guy (unregistered) in reply to Foo AKA Fooo

    How is this unreadable? The logic is pretty straightforward to read:

       For i As Integer = 0 To R - 1    ' Loop over all rows
   
        If Not DataSet.Tables(0).Rows(i)("Code") = Code Then Continue         'Skip rows unless Code matches previously identified Code
        
        If IsDBNull(DataSet.Tables(0).Rows(i)("Estimate")) Then Continue      'Skip if Estimate is null
        If DataSet.Tables(0).Rows(i)("Estimate") = "E" Then Continue          'Skip if Estimate is E
                    
        If (IsDBNull(DataSet.Tables(0).Rows(i)("Code2"))) Then Continue       'Skip if Code2 is null
                                                                              'Code2 is invalid if it contains XX, YYY, ZZZ, CC, cc, BBBB, starts with BB, or equals AAAAAA or ZZZZZZ
        If (DataSet.Tables(0).Rows(i)("Code2").Contains("XX")) Then Continue
        If (DataSet.Tables(0).Rows(i)("Code2").Contains("YYY")) Then Continue
        If (DataSet.Tables(0).Rows(i)("Code2") = "ZZZZZZ") Then Continue
        If (DataSet.Tables(0).Rows(i)("Code2").Contains("ZZZ")) Then Continue
        If ((DataSet.Tables(0).Rows(i)("Code2").Contains("CC"))) And ((DataSet.Tables(0).Rows(i)("Code2").Contains("cc"))) Then Continue
        If (DataSet.Tables(0).Rows(i)("Code2") = "AAAAAA") Then Continue
        If (DataSet.Tables(0).Rows(i)("Code2").StartsWith("BB")) Then Continue
        If (DataSet.Tables(0).Rows(i)("Code2").Contains("BBBB")) Then Continue
  • just some guy (unregistered) in reply to just some guy

    Hmm, length limit. continued:

            If (IsDBNull(DataSet.Tables(0).Rows(i)("Location"))) Then Continue    'Skip if Location is null
        Dim tempLocation As String = DataSet.Tables(0).Rows(i)("Location")
                                                                              'Location is invalid if it contains XX, YYY, ZZZ, BBBB, starts with BB, or equals AAAAAA, CCCCCC, or ZZZZZZ
        If (tempLocation.Contains("XX")) Then Continue
        If (tempLocation.Contains("YYY")) Then Continue
        If (tempLocation = "ZZZZZZ") Then Continue
        If (tempLocation.Contains("ZZZ")) Then Continue
        If (tempLocation = "CCCCCC") Then Continue
        If (tempLocation = "AAAAAA") Then Continue
        If (tempLocation.StartsWith("BB")) Then Continue
        If (tempLocation.Contains("BBBB")) Then Continue
        
        If DataSet.Tables(0).Rows(i)("Date") > LastMonth Then                 'If the current reading is later than the saved one, save the new one.

            LastMonth = DataSet.Tables(0).Rows(i)("Date")
            If Not IsDBNull(DataSet.Tables(0).Rows(i)("Present")) Then       'Missing Readings count as 0
                LastReading = DataSet.Tables(0).Rows(i)("Present")
            Else
                LastReading = 0
            End If


        End If
    
    Next

    The requirements probably evolved like this:

    • Loop over the rows, get the latest reading for the specified Code
    • Missing readings count as zero.
    • Ignore Estimated readings (flag = "E")
    • Ignore where Code2 matches blacklist: contains XX, YYY, ZZZ, CC, cc, BBBB, starts with BB, or equals AAAAAA or ZZZZZZ
    • Ignore where Location matches blacklist: contains XX, YYY, ZZZ, BBBB, starts with BB, or equals AAAAAA, CCCCCC, or ZZZZZZ

Leave a comment on “Deep VB”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article