Some shitty code I've stumbled upon

Well, it's my own little anti-showcase. Most of this probably just belongs in /r/shittyprogramming or The Daily WTF. All examples rewritten in pseudocode, to anonymise the author(s).

Implicit Polymorphism

fn foo(x: object, y: bool) {
    if y {
        do_one_thing(x)
    } else {
        do_another_thing(x)
    }
}

You most probably need polymorphism.

Cold Code

fn foo(param, param, param, value, flag) {
    if flag {
        do_one_thing(param)
        do_another_thing(param)
        do_even_more_things(param, param)
        value = keep_on_doing_things(param, param, param)
    }
    value = do_something_entirely_different(value)
    return value
}

Now about 80% of the function's body is in a big IF. How hot is this code path / how often is it triggered? Are there hidden bugs that depend on the value of the flag?

Inconsistent Interfaces

class Dog {
    id
    name
    bark()
}
class Cat {
    id
    name
    meow()
}
class Bird {
    birdName
    make_sound()
}

See how all three interfaces are slightly inconsistent, but essentially all do the same thing? What you perhaps need:

abstract class Animal {
    id
    name
    make_sound()
}
class Cat extends Animal {} // etc...

No exception, bad exception!

This code was supposed to select and parse a field from a blob; it was a tiny part right in the middle of a long and complex data processing pipeline.

The guy that wrote this crap didn't even work at the place anymore when this happened.

fn parse_foo_value(x: object) {
    if x.has_key("foo value") {
        try {
            v = parse(x["foo value"])
            if 3 <= v && v <= 7 {
                return v
            }
        } catch (ParseException e) {
        }
    }
    return config.foo_default /* default */
}

So three months later this kicks in on another data import. The "foo field" was, perhaps purposefully, renamed to something like "field foo"...

Believe it or not, it took me a week of hard work, heavy hackage, clever and precise surgeon-grade data operations, and an essay of explanations to assess and undo the damage this code has caused. If it was instead written like that:

fn parse_foo_value(x: object) {
    v = parse(x.get("foo value"))
    if !(3 <= v && v <= 7) {
        throw new ParseException("Foo out of expected range")
    }
    return v
}

...I would have caught it before any damage was done.

Another instance, different story, days of debugging:

try {
    main()
} catch (/* anything */) {
    console.clear()
}

Exceptions are not errors. They are your best friends. If they happen, they happen for a reason. Fix the problem, don't shoot the messenger.

String concatenation

This probably deserves an article of its own.

q = "SELECT " + field_name + " WHERE " + condition

Everyone knows why this is wrong, but what about this?

url = host + "/" + path + "?" + query

What if host is an empty string, and path has a leading slash? Now we have two leading slashes at the beginning of the string, and the first element of the path will be interpreted as the hostname:

//some/resource?a=1

Now of course you can test for various properties of the host, path, query variables... But all you want to do is to construct a correct URL right?

URL({path: "/some/resource", query: {a: 1}})
-> "https://thishost/some/resource?a=1"

It doesn't matter what things are you trying to concatenate, unless you are writing a low-level algorithm that must operate on raw characters, it's always a better idea to use a high-level abstraction.

goto still considered harmful

var form = null

if some_condition {
    goto ok
}

form = new Form()

if request.method == "POST" {
    if form.is_valid() {
    ok:
        return new Response("OK")
    }
}

Shitty variable names

Unless you're storing information on actual models (like model airplanes or model ships), don't use model as your variable name...

model = session.query(Users).filter(Users.name == name).first()

...and if you seriously must use it, please don't reuse the same name for an object of a different type!

model = session.query(Posts).filter(Posts.id == post_id).first()

It sucks, it seriously sucks.


See this as plaintext. Get the permalink. Check out related. Go home.