• (nodebb)

    I've been in a place where early returns were considered the epitome of evil. that's how you end up with code like this.

  • (nodebb)

    I really wish we'd stop letting engineers code without supervision. Someone should at least tell them about early returns.

    The problem is how to persuade the Powers That Be, and Those Who Decide (hopefully the same people!), of this. As I've noted before, this is something you have to construct as an argument in Businessese rather than in Engineerian - that is, an appeal based on the costs in time, in work lost (and therefore time lost to redo it), and so on. Fuss about the quality of the code later, but start by getting source control in operation to eliminate (er, reduce) the need for rework.

    At least the code is clear: if all these fields are filled, process the object, otherwise whine in a message box about the first missing field.

  • Officer Johnny Holzkopf (unregistered)

    The filename Custom.vb is terribly misleading. It should be something like SourceCode.vb - how else could you differentiate it from invoices (Invoice.xls) or reports (Report.doc)?!

  • Bogolese (unregistered)

    Yeah, we should let the AIs do the coding! :( DOH!!!

  • NotAI (unregistered)

    Imagine using a company purchased subscription to ChatGPT or the lot to compose code.
    Brand new dimensions for TheDailyWTF!

  • (nodebb) in reply to thosrtanner

    Eh, you can work around that with minor effort, assuming that convincing the other person is a lost cause:

    string message = "";

    if (condition1) { message = "problem1"; }

    if (message == "" && condition2) { message = "problem2"; }

    ...

    if (message == "") { do stuff; } else { display message; }

    return;

  • (nodebb)

    Early returns? bad bad BAD . do you not remember the early return headline "Dewey Wins" ?

  • (author) in reply to cellocgw

    The actual headline was "Dewey Defeats Truman", but what's usually left out is the defeat was in a swimsuit competition, not the election itself.

  • (nodebb) in reply to thosrtanner

    I've been in a place where early returns were considered the epitome of evil. that's how you end up with code like this.

    Not really. This code uses early returns and lots of levels of nesting. What do you think all those Exit Sub statements are doing?

  • (nodebb)

    Doesn't a default MsgBox just have an OK button? What is DefaultButton2?

  • (nodebb)

    Even without early returns, you could fix the nesting issues by simply flipping around the true/false branches and using ElseIf.

    If Item.objNamDe Is String.Empty Then
	MsgBox("objNamDe is missing or empty.", MsgBoxStyle.DefaultButton2, "Information RS2TC")
ElseIf Item.objNamEn Is String.Empty Then
	MsgBox("objNamEn is missing or empty.", MsgBoxStyle.DefaultButton2, "Information RS2TC")
ElseIf Item.artCat Is String.Empty Then
	MsgBox("Property artCat is missing or empty.", MsgBoxStyle.DefaultButton2, "Information RS2TC")
' ...
Else
	' I omitted 134 lines of logic that really should be their own function
End If
  • CompletelyAutomatedDis (unregistered) in reply to thosrtanner

    The "single return statement" rule is the epitome of cargo cult. Having seen some convoluted horrors with return statements buried in many layers of loops and if statements, I get how that rule came about, but practically always, rewriting a hard-to-follow function with multiple returns to conform to the rule without some additional refactoring just makes it even more convoluted and hard to follow.

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

    Teams that don't use version control absolutely should turn AI loose on their code IMO.

  • (nodebb) in reply to CompletelyAutomatedDis

    From what I remember the "single return" rule comes from languages that don't have structured error handling, so you'd often need to run cleanup code at the end of a function, regardless of whether there was an error.

    Hence, single return: Call functions, set values. If something goes wrong, set some error flag and release resources. And in order not to duplicate and inconsistate* the cleanup, return only at the end of the function. Maybe even throw in a "goto cleanup" to avoid deep nesting as seen here, without the (probably anyway irrelevant) performance overhead of the pattern of repeated "if not error and actual condition" blocks.

    * I don't think this word exists, but it should.

  • Hey (unregistered) in reply to jeremypnet

    Brother you are trying to argue against a bunch of... who think they know more than everyone else. Clearly most people commenting have absolutely no knowledge of what they are complaining about.

  • Craig (unregistered)

    The code looks like a WTF factory that was translated from VBA-heritage VB. In addition to the nesting, we have the following:

    • Using Object as the type of the loop iterator instead of the actual type or an interface, which leads to...
    • Using late binding on the loop iterator, which means that Option Strict is off
    • Using an Is check with String.Empty, more likely String.IsNullOrEmpty would be better (though I'll give them credit for using IsNot)
    • Using the legacy MsgBox function

    Also, a small thing, but a ContainsKey check suggests that they should possibly be using a TryGetValue type of call instead (probably there's enough going on that the double-lookup in the dictionary isn't actually worth worrying about).

  • (nodebb) in reply to davethepirate

    Doesn't a default MsgBox just have an OK button? What is DefaultButton2?

    Per the docs: "Indicates that the second button from the left is selected as the default button when the message box appears." (All the possible values for the MsgBoxStyle Enum are also described in a handy-dandy little table )

    Not sure what that does when there's only one button...

Leave a comment on “What a CAD”

Log In or post as a guest

Replying to comment #682862:

Log Out

« Return to Article