• Brian Boorman (google)

    He/She forgot a case for v_value = 0.

  • Jim (unregistered)

    I hope his unit tests got decent coverage of that function.

  • Alex (unregistered)

    Per the comments on the original post: the typo here is "ElseIf v_value < -1 Then" (rather than +1). Code of this type is seen quite frequently when dealing with legacy financial software (coughCOBOLcough), which has a nasty tendency to mix together percentages stored as integers (10%="10") and as decimals (10%="0.1") in the same data field.

    That mixing is of course the kind of thing that should never happen in a sane system (TRWTF), but once it has happened it's basically impossible to disentangle by any sane means. It is fitting, then, that this code snippet doesn't exhibit much sanity.

  • (nodebb)

    What language is this? It's not VB.NET, because that one has "Shared" instead of Static.

  • (nodebb) in reply to Brian Boorman

    Also the case when it's 23.75. Why isn't that case covered!?

  • alexmagnus (unregistered)

    At least I can imagine, how it came about.

    Probably, originally the method was used to numbers into percents and not other way around. So, the value was not divided by 100 but multiplied by it. Also, they only tracked actual percentages, so the value must have been between 0 and 1. The (value > 1) case was invalid and threw some exception.

    Then they started recordning not only percentages, but also percentage changes. The (value < 1) case was originally (value < -1) and threw the exception, as a positive measure cannot change by less than -100%. Then they noticed that positive changes above 100% are possible, and so the (value > 1) case became reguar. But instead of thinking a bit and deleting it, they overwrote it with the same expression.

    The value == - 1 case probably brought some special message originally, as that would mean whatever is measured became 0. And value == 1? Now, this one is puzzling me? Somebody clueless added it for the sake of symmetry?

    But then the method was radically changed - it became no longer numbers to percentages, but percentages to numbers. All those limits became useless (though in case there was a special message in value == -1 case needed, they had to keep it and correct to -100). But who wants to think? This just turned all multiplication signs into division signs and let it be.

    The question remains though: what was that ELSE branch for? In neither iteration of the hypothetical evolution of this method does it make any sense.

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

    Actually, the main question is W(ho)TF would write something like this?

    I don't even want to waste time asking how it came about. That's distinctly not a question that would keep me awake at night. Even the details of what it does (or does not, young Jedi) are supererogatory. I mean, the real question is, why would anybody do anything like this when asked for a simple format conversion?

    Details, you want details. OK. Obviously no part whatsoever of the switch statement makes sense. Obviously any rounding requirement (floor, ceiling, banking) is basically out of the window. Obviously the assumption that dividing a double value by 100 will give us a nice, reliable, accurate {0:00} sort of dollars and cents thing. Obviously there are better ways to do this. Obviously there are libraries for this sort of thing in absolutely every language out there. (Including Javascript, one of whose foundational errors was to have a single numerical type, ie a double.) Obviously I have missed things. Possible issues with internationalization, for example.

    This piece of dreck is a slabbering disgrace. Good to see it brought back on a bank holiday.

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

    If/Else statement. Whatever.

  • MiserableOldGit (unregistered)

    Looks like he started doing something and forgot/gave up before finishing.

    Which is a shame, as he's on a trajectory there to turning a memorable WTF into a truly epic one.

    What language is this? It's not VB.NET, because that one has "Shared" instead of Static.

    Looks like VB6 to me, but it's been a while. I'm not sure what purpose declaring static would serve in that code fragment though, there's no local variable to preserve. And i always preferred using a small purpose built class, much less prone to misunderstanding/misuse than a VB6 "static" function, which I regard as worse than careless global variables.

    Regardless of what it was supposed to be calculating, the puzzling thing is the lack of a return type on the function ... and yet it appears to be returning something. From memory I think that would have given a compiler warning, I can't remember whether you'd get an implicit void function, a type based on implication, a variant (yuk), or a variant containing a subtype based on implication (yuk squared). But then I'd never have done this anyway!

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

    I think we're getting into the esoteric here, but let's go with it.

    This does indeed look like VB6. Or 5 or 4 or maybe even 3. And one of the characteristics of VB6 is that, if you dangle an untyped variable like ConvertPercent that is implicitly converted to the type of whatever the rhs calculation might be ... and, obviously, though not in this case, that type might vary across a switch statement or an if/else cascade or even a covariant/contravariant cast ... at that point, the return value of the function is the value of the implicit variable.

    I'm in favour of this sort of shorthand when you're working with a strongly typed language like Rust. With VB6? Well, it's usually better than PHP I suppose.

    Though not in this case.

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

    On a tangent.

    In some ways, I wish the concept of "static" or "global" had never been invented. I blame Martin Richards for this, although to be fair he did teach me the basics of compiler technology via BCPL.

    But, really, we're just arguing about method dispatch, aren't we?

    Just me, but I avoid static dispatch because methods that are statically dispatched are typically bunched in a random God Object. Or, worse, in an Olympus of God Objects.

    If, on the other hand, you are (for example) calculating the cross product of two vectors ... I can't see an issue with making that a static method. The two vectors are co-dependent from the point of view of the method.

    Then again, this might depend upon whether you're aiming for a result that has no side effects, or you just don't care. And to be absolutely fair to the abortion in the OP, the only side effect that are immediately apparent is that it just doesn't work under any rational circumstances.

    Good enough for me.

  • eric bloedow (unregistered)

    reminds me of a group project i did back in college: "if x<100 do one thing, if x>100 do something else"...and the program CRASHED when x was = 100! i LOL'd and congratulated my teammate for making really good test data!

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

    I think the other problem is that "Static" (and "Shared") mean and do different things in different languages, or more specifically, different editions of VB and then between VB.net and C#.net. That's just asking for trouble.

    Oh, and I agree with your summary of the implicit typing ... methinks the variable type would be Variant(Double), but could "accidentally" end up Variant(Decimal) or Variant(Long) or something, probably not, but I was musing over return Type of the untyped function. Yes the value would be faithfully returned, assuming the compiler doesn't implicitly treat these as a void function .... I don't think it does as I remember working with code written by die-hard C programmers who insisted on writing "void" functions that way because they believed the Sub keyword to be sinful. I remember you still had to call them in a way to discard the return value, even though there wasn't one ... I'm guessing it was an empty Variant. By that logic the function return type would be Variant(xxx) so potential hilarity ensues downstream if does manage to subtype it something other than Double. Although in this case I think it would always be Double ... IIRRC the gotcha was that variantvar= SomeInt/2 would always give a variant(int) rounded according to banker's rounding. Ah the joys of people who turned Option Explicit off and relied on type conversion to do their rounding for them.

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

    I thought going through all that might lead me to some insight as to what the programmer was trying to achieve with this function. It didn't, I just feel a bit dumber for trying.

  • (nodebb)

    This looks like VBA/VB6/VB5/... (depending on how far back you want to go). A Function declared as Static effectively declares all local variables of the function as Static, that is to say the value of all local variables are preserved between calls of the function and there is only one version of the local variables, even if called recursively. Now comes the weird part: there are no local variables in this function! Clearly someone is reproducing a pattern without understanding what the parts of that pattern means. The lack of a return type means that it will be returning a Variant, which requires more room and more conversion than most other non-array types (although you can store an array in a Variant).

    As for the logic, it looks like somebody does not like (or more likely does not understand) OR operators. Hopefully, this is his/her way of performing a short-cutting OR in a language that does not have them.

    I have had to do something like this when validating input (keyboard or file) prior to storing in a spreadsheet or database to make sure the data is valid for the kind of percentage that the business logic requires. Does a percentage greater than 100% make sense to the situation? Here is a (slightly) better example, where the business rules say this percentage must be between 0% and 100%.

    ' Returns the given percentage (or number) as a Double between 0 and 1.  1 is interpreted as 100%.
Public Function ValidatePercent(Value AS Double) AS Double
    Select Case Value
    Case Is > 100
        ValidatePercent = 1.0 ' More than 100% is considered 100%.
    Case Is > 1.0
        ValidatePercent = Value * 0.01   ' Numbers in range (1, 100] are converted to decimal.  Multiply by double is faster than division by integer
    Case Is < 0.0
        ValidatePercent = 0.0  ' Less than 0% is considered 0%.
    Case Else
        ValidatePercent = Value
    End Select
  • Sole Purpose of Visit (unregistered) in reply to MiserableOldGit

    Here's more fun for you. Try converting a random 1000 line VB.Net (derived from VB6) into C#.

    I'll save you the problem of looking up a tool. Beyond question, it is github.com / icsharpcode/ CodeConverter. It's almost miraculous.

    And there are still horrible issues around implicit conversions of late-binding types, as you suggest. (I will allow OnErr Continue and the like to die a deserved death.)

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

    I'd still disagree with that approach. You want dollars and cents (as an example).

    .Oddly enough, VB (6 or .Net) works for this.

    Dim dollars = v_value as Int
Dim cents = Fix(v_value - dollars) * 100 as Int
ConvertPercent = dollars + cents / 100 as Double

    Or something like that. My point is, you can work with the language, or you can go for an obtuse and unreliable solution based upon a completely inaccurate "intuition" of the results you expect the language to provide. At least this version is easy to test and correct.

  • Dave (unregistered)

    The else isn't a wtf. If you have e.g. a text string that some idiot's entered instead of a number, return that.

    Why you'd have bad inputs you don't check is another matter. But it might not be a wtf to have an input box in which, say, users can enter progress, or words like 'pending', and you want to either convert the input to a percentage or display the note.

  • Luchs (unregistered) in reply to Dave

    I'm not sure, but if you're checking for input validity, you could just use "IsNumeric()" in VB, can't you? The parameter is still Double typed, so you wouldn't (technically) encounter a String there either way.

    To me, it looks like one of those companies that measure "productivity" by "lines of code".

    "Programmers write code. The more code they write, the more productive they are, so le's measure this!" with the extreme end being "Write 200 lines per code per day or you're fired". This would explain the "WTF" quite well.

  • David Mårtensson (unregistered) in reply to Brian Boorman

    I do think <1 covers both 0 and -1 and with >1, =1 and <1 your left with irrationals or similar to reach the last else :)

  • MiserableOldGit (unregistered) in reply to Dave

    What kind of string are you going to manage to shoehorn into a parameter explicitly declared as "Double"? FILE_NOT_FOUND?

  • WTFGuy (unregistered)

    When I do a multi-case elseIf / switch / select / etc. code block, I always include a final case else or default: even when the prior selectors are supposed to be collectively universal. The contents of that final case is a comment "shouldn't ever get here" and the equivalent of throw new NotImplementedException() or similar.

    The whole point is to make explicit the idea that the previous case selectors are supposed to be universal and any changes to them that alters that assumption is a breaking change. It tells the next guy: "Yes, I thought of that, and here's the answer." The usual use case where this saves bacon is when the wall-of-elses is a long laundry list of business-rule driven special cases. Then the bizidjits decide that case 14 is no longer applicable so the hapless maintenance dev located case 14 and deletes it. Unfortunately there's still case 14 data out there. Throwing the unexpected exception isn't necessarily business-correct behavior, but it avoids permitting widespread unnoticed business-incorrect behavior.

    It also covers for the all too common situation where some upstream caller somehow sneaks an unexpected or invalid value past the validation. The more mixed language and mixed ancient vs. modern your codebase, the more of those logic leaks slip in.

    It also catches screwups if you're selecting on the various values of an enum and somebody later adds a new value to said enum, followed by various upstream code using the new values while your code is unchanged. Again the goal is to make the failures loud, not silent.

    TLDR: defense in depth applies to more than just security. It also applies to correctness.

  • MiserableOldGit (unregistered)
    When I do a multi-case elseIf / switch / select / etc. code block, I always include a final case else or default: even when the prior selectors are supposed to be collectively universal. The contents of that final case is a comment "shouldn't ever get here" and the equivalent of throw new NotImplementedException() or similar.

    I agree with that, although this particular example looks like it would result in weird downstream numbers rather than a call to action. Except it wouldn't unless a careless edit is made and, again, the way the approach is implemented is risks disguising a new problem rather than highlighting it.

  • MiserableOldGit (unregistered) in reply to Sole Purpose of Visit
    Here's more fun for you. Try converting a random 1000 line VB.Net (derived from VB6) into C#.

    Needs more excitement ... tell me the VB6 was a port from AccessBasic via Excel VBA, which itself was an add on to an 80s extinct 4GL platform, which still exists and needs to maintain datalinks.

    And the client thinks you are just making this hard because he heard on a "course" he went on that Machine Learning can do all this without the need for picky code monkeys asking awkward questions about range checks and data type mismatches.

  • Uncle Bob (unregistered)

    Textbook example of the TDD methodology - keep adding cases until the tests pass.

  • Thomas (unregistered) in reply to MiserableOldGit

    In some languages like C#, Double could also have a value of Double.NaN, which is neither >1 nor <1 nor =1, because it's literally Not a Number. So this last else could make sense in that regard. Whether it is this case is another story.

Leave a comment on “Classic WTF: Covering All Cases… And Then Some”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article