Shit programmers write(shitprogrammerswrite.com)

63 pointsby mat-mcloughlin13 anni fa77 comments

http://shitprogrammerswrite.com/

17 comments

collyw13 anni fa

A lot of these seem to be attempts to fit into existing "best practices". Unit tests that prove nothing. Boilerplate Java code that does nothing.

Maybe we should reevaluate some best practices. I have debated before on here that many unit tests seem useless as the units are too small, and you essentially end up testing your language or framework which you already know works. Integration testing on the other hand makes a lot more sens.

When I started with Java, it seemed appropriate to put hundreds of getter / setter methods. Is there much advantage to that over allowing the variable to be accessed directly? (Perl felt very strange at first having getter and setter methods combined as one method).

As I get more experienced as a developer, my skill set has grown but my coding becomes simpler. Don't use every language feature to show how knowledgeable you are. Use it when appropriate, and when it makes the code more readable / reusable / simpler. Sometimes a higher level abstraction is more difficult to understand , but is overall better choice. An example would be a map as opposed to a for loop. Less chance of side effects in a map, as we don't have to track the iterator variable, but a map is not as intuitive for less experienced coders.

mbillie113 anni fa

The fact that you can write bad unit tests is orthogonal to the notion that writing unit tests is, in general, a good thing to do.

sanderjd13 anni fa

I always feel happiest writing unit tests for functions that contain non-trivial logic. But I always feel sad when testing methods that mostly just introduce side effects. It seems like when I'm running into more of the latter, it is often worthwhile to find ways to refactor toward having more of the former, but sometimes not.

wlievens13 anni fa

True. I find unit tests the most useful and economical when I'm testing algorithms, rather than data bookkeeping or user interfaces.

pan6913 anni fa

Best practices is something you learn over time, as you gain experience. I don't think there is a lack of best practices, it's just that there are always developers learning and gaining experience and before a best practice to make sense to them they have to do it wrong first. I.e. knowing the rules of chess doesn't make you a chess master.

As I mature as a developer I notice that it takes more and more time for me to finish something. When I was younger I simply wasn't aware of half the stuff that could go wrong. Now I'm older and I am aware I find myself taking more and more time to implement something and spend a lot more time on e.g. clean interfaces and error handling. Something I simply didn't do many many years ago.

reginaldjcooper13 anni fa

> When I started with Java, it seemed appropriate to put hundreds of getter / setter methods. Is there much advantage to that over allowing the variable to be accessed directly?

Oh my yes. As soon as you need more than one processor messing with an object they are invaluable. The only thing better is immutable objects.

It's also nice to have the object interface separate from object data if you want to change the object data representation without breaking everything (I don't know in general when you would do this, I just know I have done it before and getter/setters let me work faster to refactor).

sbierwagen13 anni fa

Little counterproductive to censor the submission title when the domain name is right next to it.

mmahemoff13 anni fa

More frequently updated: http://thedailywtf.com/

flatline13 anni fa

A few of these are funny/wrong, but most just look like laziness or some minor confusion about language/framework specifics. Thedailywtf definitely has better content.

danabramov13 anni fa

To be more specific, http://thedailywtf.com/Series/CodeSOD.aspx has little snippets like these, but way funnier.

ep10313 anni fa

public bool ShowOptional() { bool bolReturn = false;

    return bolReturn;

}

The above actually seems reasonable if you assume the programmer that wrote it was intelligent. I'm imagining that the above code is called multiple times by the application, for a new optional feature in development. Currently, they haven't developed the feature, and so we are always returning false. But in the future, we likely will want to show the feature given some condition. So this programmer has (hopefully) decided that (s)he will write the current code to take this future development into account, and rather than pass some boolean or config value throughout the code, has isolated it to one specific method.

Now, when they go to implement Optional, whatever that is, that developer can just update this one method with the expression, and go about coding their feature without any knowledge of the previous code base, which is exactly what's supposed to happen.

michael_h13 anni fa

Why have a variable then?

  return false;

Done.

pmelendez13 anni fa

I used to write this kind of snippet when creating the skeleton of a function, this seems like a leftover that was over-sighted in the code review (assuming that there was a code review at all)

pc8613 anni fa

If that’s the case shouldn’t they have just used

    public bool ShowOptional()
    {
        return false;
    }

?

MattBearman13 anni fa

Love this one:

  return foo == null ? null : foo;

Peaker13 anni fa

I don't think people tend to write this kind of code, per se.

Rather, it "evolves".

It starts as:

  return foo == null ? error_handler(bar) : foo;

And someone realizes error_handler actually does nothing, so replaces all calls with null, mechanically.

ctdonath13 anni fa

This is often the case. When implementing something complex, a train of thought may require writing code then reducing it down to a degenerate form once certain conditions are confirmed/discovered; often the degenerate form is left as a placeholder in case something need be added back in ... alas, the final cleanup stage may not arrive and some oddities may be left behind to confuse future maintainers. Cleverness can, from some angles, look stupid.

Or maybe it's just stupid.

Sometimes you just can't tell the difference.

werner3413 anni fa

Exactly.

The same is probably true for this one here:

public bool ShowOptional() { bool bolReturn = false;

    return bolReturn;

}

There should be descriptions why the code snippets are supposed to be interesting, your and my example just look like legacy code to me.

sebastianmck13 anni fa

That one makes a bit of sense in Javascript as it'd return null if foo was null OR undefined.

surreal13 anni fa

It might be technically correct, but if the intention was a loose comparison surely don't include null in the comparison?

  return !foo ? null : foo;

surreal13 anni fa

... or

  return foo || null;

if you're into that sort of thing

wreegab13 anni fa

This reminds me of this one, which is quite common:

   return foo == null ? true : false;

diaz13 anni fa

I did something of this kind recently and it was to enforce more of an api in my object, like internally I call some low api method that either gives me the object I'm looking for or false / null when doesn't find it, but in my interface I don't want to leak that object, just true or false. So

function isAvailable(){ objInDb = findByName('Joan Carlos'); // the object or false

   return objInDb ? true : false;

}

But all this could be because I'm a noob in a language I don't really know... Php that magical land where nothing is what it looks like and is always ready to stab you in the back, can't say I'm a fan of it... Or dynamic languages, or anything magical... Ok, I'm going to places I don't want to remember, sorry...

EpicEng13 anni fa

I this case, returning objInDb would be semantically different. So, if you always want to return a Boolean, you did the right thing.

Leszek13 anni fa

It's better than

    return foo ? true : false;

or even

    return foo == true ? true : false;

wreegab13 anni fa

So transposing a boolean into a boolean is better than transposing a truthy value of an unspecified type into a boolean?

You are missing the point, which is it's really silly to transpose a boolean into a boolean using a ternary operator.

If you want to argue that "foo == null" is the problem due implicit nil value falseness, than the solution is simply to use strict comparison operator "===", transposing "foo == null" into a boolean doesn't solve the falseness issue if you think this is what the original author was trying to address.

[deleted]13 anni fa

adamors13 anni fa

I sometimes do that in PHP because I hate the equivalence between "", false, null, '0' and 0. Seeing true and false written out makes it more readable.

sneak13 anni fa

When you find your language forces you to do stupid things, the correct response is not "get in the habit of doing stupid things". It's "find a better language".

wreegab13 anni fa

The result of "foo == null" (or better "foo === null") is boolean, I don't know what is the "equivalence" problem here.

sixothree13 anni fa

Maybe this is a programmer who likes to spell things out for future readers. It's easier to convey meaning with return foo == null ? true : false;

wreegab13 anni fa

Let's picture a future reader:

"Hmmm, if 'foo == null' is true, than it's true... I see..."

Seriously..

sixothree13 anni fa

Yeah, it was a wild guess. I have a habit of making things more verbose for future maintainers.

jebblue13 anni fa

From a maintenance perspective, it's a bit more involved than that. Returning the explicit evaluation result says, "I do care what foo is and these are the only valid possible return values after expression evaluation, today just return null but in the future we might return something different, either way the return value at this point is critical and needs to be explicitly considered and processed".

EpicEng13 anni fa

To a non programmer or an absolute beginner? Sure, but then I would ask; why in the hell are you writing your code in a style catered to people who don't know what they are doing? That line simply shows that the writer doesn't understand Boolean expressions.

collyw13 anni fa

would a comment not be more appropriate in such circumstances?

smikhanov13 anni fa

If this:

  function nop()
  {
      if (Math.random() == 0.1234567890)
      {
          for (var i = 0; i < arguments.length; i++)
          {
              console.log(arguments[i]);
          }
      }
  }

wouldn't have been written in JavaScript, my guess would be that this is an attempt to create an empty function (no-op) and prevent it from being optimized away by the compiler. Still unclear who may need to call it.

hrktb13 anni fa

> if (Session["startDate"] + "" == "")

This feels horribly familiar. I'm not sure to recognize the specific language used, but there must be cases where one would like to target empty of filled with non processable characters strings only, and let null and falsy values pass through. This kind of use would typically need a line of comment, but hey...

goldenkey13 anni fa

I'm guessing you don't realize that the plus operator has less precedence than the == operator.

That if is equivalent to if (startDate + true)

EDIT: I was wrong. I haven't slept.

kirab13 anni fa

Sadly you are wrong. See an operator precedence list: http://bmanolov.free.fr/javaoperators.php

goldenkey13 anni fa

You are right, sorry about that.

krcz13 anni fa

Does it mean if(1 + 1 == 2) is equivalent to if(1 + true)?

kirab13 anni fa

1) + has a higher precedence than ==

2) even if == had a higher precedence, then 1 == 2 would still be false :D

goldenkey13 anni fa

I was wrong, my bad.

statictype13 anni fa

Do this in javascript all the time. Adding a `+ ""` forces the object to become a string.

Though not sure why it was required for this particular comparison.

rpedela13 anni fa

Why not use toString()? It is clearer to the reader that way.

Eiwatah413 anni fa

toString() blows up on a null value, the + operator does not.

I've written similar Java code for that reason.

danielweber13 anni fa

I shell languages I do this a lot.

    if "x$VARIABLE" == "x"

because it handles the cases where $VARIABLE is undefined or multiple words without vomiting all over the place.

viraptor13 anni fa

I hate when people do that because it's not needed at all. The quoting already fixes everything.

    if [[ "$VARIABLE" == "" ]]

or even better

    if [ -z "$VARIABLE" ]

will handle undefined, empty and multiple words just fine.

twic13 anni fa

Unless you're running with -u, which you should be, in which case it will blow up on an undefined variable. To treat undefined as empty, you want:

    if [[ -z "${VARIABLE:-}" ]]

fdej13 anni fa

    if (Math.random() == 0.1234567890)
    {
        for (var i = 0; i < arguments.length; i++)
        {
            console.log(arguments[i]);
        }
    }

Reminds me of the code I have to insert to prevent overzealous compilers from completely optimizing away my benchmark loops.

pmelendez13 anni fa

I haven't slept well lastnight (only 4 hours) so please bear with me... but what is the joke on this snippet?

"I Don’t Know".ToJson();

    public string ToJson()
    {
        var s = new StringBuilder("{");
        for (var i = 0; i < CustomField.CustomFieldOption.Count; i++)
        {
            var item = CustomField.CustomFieldOption[i];
            s.Append("\"" + item.CustomFieldOptionId + "\"");
            s.Append(":\"" + item.OptionName + "\"");
            if (i < CustomField.CustomFieldOption.Count - 1)
            {
                s.Append(",");
            }
        }
        s.Append("}");
        return s.ToString();
    }

acron013 anni fa

There aren't any obvious structural errors but two things occur to me. Firstly, JSON serialization is something every platform, including .NET, already has a multitude of libraries available for so this is 'reinventing the wheel' a bit. Secondly, looking at what it's actually doing, it's plausible the author doesn't even need JSON, as it's simply serializing a list of `id:name` fields. Thirdly, there are plenty of more 'inline' ways of achieving this using `string.Join`. All in all, just very 'novice' code but nothing _incorrect_ about it. IMO.

pmelendez13 anni fa

Yeah, that's what I thought. I didn't see anything horrible that were obvious, and by horrible I mean laughable just as the other pieces of code in the website.

jtheory13 anni fa

There are easier ways to "JSONize" data, maybe? They aren't escaping the keys/values going in, which could be bad, but if it's a known set of strings that may not actually be a problem.

I notice that the two pieces of data actually going into the JSON are "CustomFieldOptionId" and "OptionName" -- which seem somewhat useless/obscure, but there could be a legitimate reason for that.

So yeah, I don't quite get it either. Most of the others are more obviously silly.

philliphaydon13 anni fa

Nope, 100% real. Copy pasted as is from a project I work on. Legacy code from 5? Years ago... No idea who the original author is. But it worked. It now uses newtonsoft.json library now.

About 6 years ago I wrote something similar, but that was before newtonsoft.json was super popular.

iron_ball13 anni fa

I'm assuming they could have just used a well-known and well-tested library.

mephi5t013 anni fa

I assume item itself is missing, so nothing to convert... My guess

potatolicious13 anni fa

Been a while but:

  var item = CustomField.CustomFieldOption[i];

Shouldn't that be the item to convert? This looks like something stuck onto a class to serialize to JSON.

I might be missing the joke also :(

[deleted]13 anni fa

(1)

pron13 anni fa

I wonder how much of that is due to problems in documentation/documentation discoverability. This seems precisely the kind of data that we need to make docs better.

npsimons13 anni fa

Or, you know, write programming languages that don't suck. See also https://www.destroyallsoftware.com/talks/wat

JoeAltmaier13 anni fa

The one about if (condition) {...} else if (!condition) ... May actually have a point, if 'condition' is a boolean which can be true or false or undefined.

[deleted]13 anni fa

jtheory13 anni fa

For the "Siamese HTML Document" -- that one rings a bell... I swear there was a bug in one of the ancient horrible versions of IE that required you to include your "no cache" tags AGAIN in a second head tag, if you wanted to use http-equiv tags instead of real HTTP headers.

I'm pretty sure there hasn't been reason for anyone to do that for a decade or so, though.

[deleted]13 anni fa

slaman13 anni fa

Is anyone else as infuriated as I am that you can't actually read any of the code?

Maybe I'm missing a button somewhere, but I needed to open up the DOM inspector or RSS feed to read every single one.

Unacceptable

tkellogg13 anni fa

A lot of these seem to be Microsoft technologies like C# or SQL server. Is that because the owner is more familiar with the MS stack? Or do MS devs have more to complain about?

danabramov13 anni fa

From my experience, such code is not specific to MS technologies, but is indeed often found in “enterprisey” code, a lot of which just happens to be in Java and C#. And enterprisey code is like that because it's often outsourced to the lowest bidder.

I once had to print a method that took 50 pages of paper, so I could understand what it does, and after 15 minutes I realized it's the same 60 lines repeated over and over with different conditions. The guy who wrote it had no idea he could have extracted a method.

morganherlocker13 anni fa

I replaced an old AS400 system from the 70s as one of my first gigs. It was 50k lines of incomprehensible RPG code. I would often have to print out the code, steal a conference room, lay out all the pages, and go to town with a bunch of highlighters to tell what the hell was going on.

On the plus side, the code still had comments for the punchcard numbers, so it was pretty easy to keep the pages in order :)

guardian5x13 anni fa

Just curious: Why does printing it on paper helps you to understand code better than reading it on screen?

danabramov13 anni fa

It helps to understand the structure. I was looking for repetitions (and luckily I found a lot), but I needed to look out for things that change between repetitions.

I just found the picture I took that day. The boxing gloves just happened to be in the same conf room.

http://i.imgur.com/bQCA08C.jpg

hamburglar13 anni fa

Looking as close as I can at those printouts, my only reaction is, "that's CODE?!" Good lord.

zentiggr13 anni fa

Every once in a while you find code whose control structures are wrapped around a ton of working code... paging up and down to try to grok the entire structure pagefaults my short term memory badly enough that having it all visible at once is the only workaround.

Sometimes, if you're really lucky, the blocks of working code don't modify their own conditionals, and you can 'fold' them and just look at the if's and loops...

pseut13 anni fa

For me, the ability to spread the paper across a table and look at many different parts of the code simultaneously. It's like having a giant high resolution monitor.

Also, faster and more flexible annotation (drawings, etc), but that's secondary.

MartinCron13 anni fa

I've only had to print terrible code to understand it. I think it's a way to leverage the spatial part of your brain more directly.... where was that part of this huge method that did that... oh, it's top left.

walshemj13 anni fa

Because if you can find a room with enough table space you can lay out all of the code at once instead of having to jump back and forward on a small screen.

A 4x6 table has far more surface area than even the biggest multi monitor set up

ep10313 anni fa

This is my life.

philliphaydon13 anni fa

I'm not the owner, but I know the owner of the site. It's mostly C# / SQL because he is a .NET developer and so its only really been shared among other .NET devs.

It's not meant to be specific to any language. It's just funny shit people have written.

I submitted a few from old code I wrote like 5 years ago. Its all good fun :)

walshemj13 anni fa

Oh I thought the shit programmers write was stuff like "its; 90% complete" or "thats not a bug its a feature"