Excellent XKCD (and crappy code)

[Thorin]

Today's XKCD is about homegrown, ugly code:



source: http://xkcd.com/1513/

I'd just like to say, this doesn't only happen to people who are self-taught...

[Mr. Analog]

What's the difference between this:

Code Select Expand

if (blah) {
//Do amazing thing
}

and this

Code Select Expand

if (blah)
{
//Do amazing thing
}


...right... and yet there have been endless wars raging since the dawn of time about code style.

In my world: Ctrl+K,Ctrl+D

This is one of the low-grab XKCDs, if this isn't about petty formatting it's about developer dick waving "you did this ALL wrong". That said we've all had those "WTF was this developer doing this is all backward" moments, what would have been funnier is if the dev in the comic discovered it was their own code from 2 years ago

[Thorin]

how about the difference between

Code Select Expand


if (varA) Response.Write(varB ?? false ? "Yes"
                      : "<br />not in a million years");


and

Code Select Expand


if (checkForLastManAlive)
{
    if (willNotSleepWithYou == null || willNotSleepWithYou == false)
    {
        Response.Write("Yes");
    }
    else
    {
        Response.Write("<br />Not in a million years");
    }
}


Now which of those is easier to understand?  And that's just a simple example of the type of messed-up, hard to read code I've seen in my years.  It's one of the downsides for working at small development shops where the number of developers spanned between two and four, and at shops where they hired an outside company whose developers were more interested in creating a custom web server than actually creating an application...

[Mr. Analog]

TBH both were readable, one was better though for sure

My main yelling point when it comes to Bass Ackwards Code is when somebody makes a ton of work for themselves rather than doing 15 minutes of reading

Or people using CLEARLY NAMED properties for something else because they didn't want to add a new property

Or people insisting their code is right and changing an OPEN SOURCE resource without realizing that IN DOING THAT we'd have to ALSO open our code base

Or people using hard coded SQL strings that are more "efficient" than the ORM we use (well duh) rather than writing a SPROC because reasons

Believe me I get THE ANGER, I just... bleh

Sometimes Randall Munroe is SO CLOSE to making something absolutely hilarious but then misses it my just a smidge, and it rubs me the wrong way.

Like imagine if the angry dev was saying those things in a _certain developers_ voice ("Weeeeeeeeeeell") and it takes on a whole new meaning

[Thorin]

See, I don't see an angry dev in that comic.  In the first panel, the non-self-taught dev is clearly happy to look over the code, and then from there it seems more like they're just completely amazed at the level of spaghetti rather than that they're angry.

I think the most obtuse code I've ever seen was that custom Java web server - I stepped through 47 classes to figure out what text was going to be displayed in a label on a web page. 21 classes in the web app, then another 26 classes in the custom Java web server.  It was just bat@%&# insane.  I found it easier to understand the shortest path routing engine I had to fix up, which used advanced mathematics to find the shortest path in a set of over a million paths in 500ms or less, with random (user-provided) start and end nodes.

[Mr. Analog]

haha I remember that!

Also remember the solution that wouldn't compile without the other solution compiling first, which had a circular dependency to the other solution

THE CIRCLE OF DEATH lol

[Thorin]

Yeah, I figured out later that the person who wrote that would compile each project in the solution separately, changing the compiler directives for certain projects between compiles, and then combine the dlls by hand into a distributable package.  And of course, nothing about that was written down.

[Mr. Analog]

Hahaha

People building Stonehenge: "We'll document it later"

[Darren Dirt]

how about the difference between

Code Select Expand


if (varA) Response.Write(varB ?? false ? "Yes"
                      : "<br />not in a million years");


and

Code Select Expand


if (checkForLastManAlive)
{
    if (willNotSleepWithYou == null || willNotSleepWithYou == false)
    {
        Response.Write("Yes");
    }
    else
    {
        Response.Write("<br />Not in a million years");
    }
}


STRAW MAN ALERT!

Most of the above "bad code" example is simply not-self-documenting code; you could take the latter's readability and compress it *if that helped in some way and did not destroy maintainability*. The "some way" is up to the developer and/or their team to decide if it's worth the reduction in clarity.

I think the XKCD is more likely making reference to weird approaches to solving problems, weird non-standard styles (or old "legacy" mindsets) that are less efficient or do not scale well, compared to more modern design patterns etc.


Or even worse, maybe it's mocking the stuff I encountered when I started at my current job, a ton of code was like

[do thing for field_name_1]
[do thing for field_name_2]
[do thing for field_name_3]
[do thing for field_name_4]

where "thing" was like 20 lines of code. Yes, that's right, the original code had like 80 lines of code instead of wrapping a "for field_suffix = 1 to 4 ; next field_suffix" wrapper around just 20.

Stuff like that, maybe, is the target of this comic? Because that original coder "didn't know any better" (and likely had no time or motivation to refactor it into something not insanely inefficient, lol).


Sometimes we have blind spots that we don't even recognize after others point them out, let alone realize them on our own. 

[Thorin]

I think it's more poking at the kind of code you get when someone squishes a do-while loop and a case statement together and it works even though trained programmers would all agree it should not.

For instance, this is valid C:

Code Select Expand


int n = (count + 7) / 8;
switch(count % 8) {
  case 0: do { *to++ = *from++;
  case 7: *to++ = *from++;
  case 6: *to++ = *from++;
  case 5: *to++ = *from++;
  case 4: *to++ = *from++;
  case 3: *to++ = *from++;
  case 2: *to++ = *from++;
  case 1: *to++ = *from++;
} while(--n > 0);
}


But what the hell does it do?  And how in the world does it manage to have a do-while loop interlaced with a switch?

(for the astute, yes, that is Duff's Device)

[Darren Dirt]

^ if your code is so unclear that it only makes sense if someone hand-builds a "state table" of its initial few conditions and ESPECIALLY if it works by relying on pointers and ++ and -- and missing "break" commands, then you're doing it wrong (or at least doing it more "tricky" than your future maintainers would prefer. Including possibly YOU!)

imho.

[Darren Dirt]

On topic with the OP and some of the discussion:

http://butunclebob.com/ArticleS.TimOttinger.ApologizeIncode
"A comment is an apology." (that's how this perhaps-contrarian article begins )

SEE ALSO: http://c2.com/cgi/wiki?ToNeedComments
(which starts with "Refactor the code properly and you won't need comments.") ...and gets rather heated rather quickly...

[Mr. Analog]

I find the "handled by source control comments" comment to be null

Source control changes more often than the codebase and if the source history isn't somehow lost then the little-to-no comments are generally useless (fixed to resolve issue with bug #55290856 <-[that system also gone])

The only comments I really want to see are when somebody thought they needed to something weird and it's not explicable by code alone

Code Select Expand

//This version of library X treats this property incorrectly, detail below

9 times out of 10 I don't care, unless I see something really ratty and then I want to know what was going through that devs mind beyond "I just learnt what a Tuple is and I'm going to use it EVERYWHERE")

[Darren Dirt]

A comment is an apology for not choosing a more clear name, or a more reasonable set of parameters,
or for the failure to use explanatory variables and explanatory functions.
Apologies for making the code unmaintainable, apologies for not using well-known algorithms, apologies for writing 'clever' code,
apologies for not having a good version control system, apologies for not having finished the job of writing the code,
or for leaving vulnerabilities or flaws in the code, apologies for hand-optimizing C code in ugly ways.
If something is hard to understand or inobvious, then someone *ought* to apologize for not fixing it.
That's the worst kind of coding misstep. It's okay if I don't really get how something works so long as I know how to use it,
and it really does work. But if it's too easy to misuse, you had better start writing.

...somebody's grumpy... (but mostly correct)