If you have ever written software, then you will most likely find this to be funny. Despite the joke on programmers here, I really have made it my goal over the years to write less code. The huge block of code that the “seasoned professional” uses to print “hello world” actually makes me angry. I know this is a joke, but I’ve seen coders who acted this way and I’m always horrified when I encounter their work.

It’s like having a twenty-page document on how to fill a Pez dispenser. The underlying functionality is simple, but the instructions are so verbose that if you want to make a change you’re better off dumping the original and writing the whole thing over from scratch.

In my day job I often work with a large-ish codebase (I’m gonna guess it’s somewhere in the ballpark of two or three million lines of code) which has been around since sometime in 1994. It’s crufty now, and once in a while I’ll need to go into one of the old, dusty corners of the codebase and work on it. I can often tell who wrote the code just by looking at it.

Sometimes I’ll have trouble getting a non-coder superior to understand why we need to throw away (or re-write) a block of functional code when it apparently “works just fine” and all we need is “a minor change”. This is particularly true since I’m reluctant to be critical of my peers and very reluctant to be critical of peers which have departed for other jobs. It’s a very weasel-ish practice to point the finger of blame at a guy who left two years ago, even if the work he left behind is borderline sabotage. So I can either blame a guy who isn’t around to defend himself, or I can act as caretaker for his psychotic, unruly code.

I took some courses in HS and college in programming, so I had a bit of foundation laid. My current job sent several of us downstairs to learn VBA. Now we often end up fighting with someone else’s code, which is fine, except for the guy who rocks all of our programming worlds, so much so, that he refuses to document any code or give any variables or sub routines descriptive names.

Me: “Hey, John, which macro do I run to do XYZ?”
Him: “Um. . .try ‘ABC123′.”
Me: “Grrrr…”

I’m taking Visual Basic. I tried to switch up to Java, since I’m already learning it, but there was simply too much red tape to cut through.

My programming teacher, who has been ‘teaching’ (I use the word loosely) for 6 years makes us write all of this redundant code. For example, Coding for two checkboxes:

If chk1.value = 1 and chk2.value = 1 then
‘Both are checked
Else if chk1.value = 1 then
‘One is checked
Else if chk2.value = 1 then
‘Two is checked
Else
‘So on
End If

It would be something like a price, and instead of two checkboxes, it would be four or five. It angered me to no end to have to write over 15 lines of code for what could be accomplished in no more than 5, but he insisted that ‘His way is the right way’.

Sadly, the options regarding departed staff tend to boil down to:
1) They were wonderful and exceptional people, and no-one else is talented enough to understand how it works;
2) They were useless wastes of oxygen, and their work is crap.

I know how you feel. I’m a junior programmer at my workplace and I am constantly trying to convince my older colleagues that they need to trim down their code. You don’t need 20 million lines of code to select from a database and show it on a webpage. :) I’m with ya!

I remember seeing that site a while ago, it’s amusing, primarily because it’s so true. My second Java teacher (200 level) was particularly bad at this, she always had error checking for even the most trivial of tasks…and she did this before teaching the class what error checking was (so basically all her example code was about 10% lines that the majority of the class couldn’t understand as they hadn’t learned it yet, and that the rest knew to be pointless and a waste of time.)

It almost seems like most of the good programmers want to write code that’s unreadable by someone not up to their level or something, and with higher level programming languages that becomes difficult unless you include a lot of meaningless code.

That, or they’re all trying to with the International Obfuscated Code Contest or something :P.

Each function named something like “StringFunction1″, “StringFunction2″, “IntFunction1″, “intFunction2″, etc.

Each parameter was actually a variant encoded as a string, and intuitively named… you guessed it: “parameter1″, “parameter2″, “parameter3″, etc.

There’s nothing for building coding discipline like seeing some of the nastiness that’s actually out and about in the world. My fervent hope is that I will never manage to be such an object lesson for anyone else.

Binks: That is my pet peeve: Example code with layers of checking, setup code, cleanup code. Yes, a real program needs all of that, but not a tutorial. It’s like teaching someone to use a hammer by giving them the blueprints for a house.

As far as convincing your managers you need to “re-write” a section, That was a quote from a book I ran across: “There is a defined point at which a block of code can not be modified further without introducing a bug”. Their argument was that scope creep should be applied at a code block level.

I’ve been involved in game industry almost all my life, even if I don’t do it for living. Subsequently, I’ve had to deal with coders, and one of my closest friends have been “talking code” to me since he started learning it some 13 years ago, and talking like I was just another coder. The interesting result of this is that I know many programming techniques, I can actually read C (assuming it’s actually written in comprehensible way), and I’ve actually come across this kind of code-bloat where it would take me ages to understand what the f*ck the code is for, and then seeing that it actually does almost nothing.

Blurr: I like your simplified code. However, for readability it’s not so good. Reading your efficient code I don’t immediately get that chk1 = 7, chk2=3 and chk1+chk2=10. *I* would get it after a second glance at the code but my colleagues would take a long while to pick it up (usually ending up with them in my office asking what it does).

In my case, I often use Trinary operators (instead of writing out an if/then/else structure in some languages you can use this syntax (if)?then:else ie. (chk1=1)?cprice=3:cprice=7). They can get quite complicated as you can have nested conditions. I often start with a trinary operator (or other simplifier) and then convert them into a more readable format later.

Great stuff! Although the High School/Junior High example doesn’t seem to be complete without the “15 GOTO 10″ thrown in there.

I’m at the point now where reading my colleagues’ code actually causes me pain. At some point, bad crufty coding practices that have been allowed to exist for too long get institutionalized — passed around as examples for junior people to read and learn from, or “reused” as the baseline for a new script. So the longer that inefficient code is allowed to exist, the greater the possibility that it will actually start replicating itself. Beware the spawning cruft monster!

Inept predecessors seems to be a pretty common problem in the software world. (Or, worse yet, inept coworkers!) I feel like the issue is derivative of the phenomenon described in this post on programmer duality on Coding Horror. The 20% are constantly cleaning up after the 80% and it decreases overall productivity, but I think Shamus has a good point about restraining the urge to badmouth inept colleagues; if you do manage to stay positive, things tend to stay much less tense, and “the 80%” are perhaps more likely to pick up new techniques.

Several years ago the University I work at hired an outside group to write the backend code for the main web site. Which I’ve inherited, of course. Every time I have to look at it I’m amazed at how well they’ve obfuscated their code. Includes including includes including includes. Hundreds of files all loaded by every page every time someone visits our site.

A couple of months ago when the server was having one of its many load tantrums I went into two of their major include-include files and just started commenting out includes until things broke. I was able to remove about half of them without anything breaking; I also focussed some functionality (every page on the site doesn’t need to load the job posting database).

The kicker came when I started the upgrade process to PHP5. Their code broke, deep in one of the real files. My first thought was, how did they see the future to know how to write code that would break, because I swear it had to be deliberate.

Turns out they’re using an undocumented “feature” that no one in their right mind could expect to work reliably or predictably. Inside of a loop in some of their object methods they’re resetting the current object to something else–$this = $newobject — and just continuing on in the loop.

I’m not even sure how to analogize this for non-programmers. Maybe it’s like switching blueprints halfway through building a house without telling anybody. Or building a house on a hill–and then calling in excavators to remove the hill. What is this supposed to do? Does the current method swap out? What happens to the old one? What happens to the current loop?

So I was able to use the upgrade as an excuse to rewrite their hundreds of files into about four small files that we’ll be able to understand.

Shamus, I’ve found that finger-pointing causes resistance to change because it causes people to focus on the person rather than the problem. The best way to overcome the resistance is to shift the blame to a defective system or process (following Deming’s assessment that only 5% of problems are caused by individuals; the rest by defective management-owned processes or systems). One way would be to follow Mike R.’s advice and incorporate, as a standard part of the code maintenance process, a case-by-case assessment of the code robustness. Another, similar approach, would be to develop well-justified programming guidelines–I would suggest separate usage and style guidelines–and then evaluate code against them. When code doesn’t measure up, you have justification for rewriting without ever mentioning previous developers. Applying such approaches more broadly (i.e. applied to new code development) should have the added benefit of resulting in better code and more consistent code across developers.

#15 Rob has a potential bug where he assigns the value 1 to chk1 when he writes: (chk1=1)?cprice=3:cprice=7
This presumes that = is an assignment.

I’ve defensively learned to put the constant first, e.g.
(1=chk1)?cprice=3:cprice=7
so the compiler will flag an error if I screw up my test vs. assignment operator.

I’m a bit uncomfortable with using the trinary operator, though it certainly has its place. But personally, I think it makes the code harder to read — I have to *think* about it more. But that’s probably why you switch to a simplified format later.

One of the harder lessons that I’ve learned is that programming for values that shouldn’t make it into internal functions increases the number of bugs I have to deal with. It handles the wrong input gracefully, thus hiding the bug and making it harder to track down. If it instead chokes and throws a stack dump, I know about the problem where I would have been unaware before.

There’s also the usual bag of tricks such as putting the constant first in comparisons. I also like always putting an explicit true or false into if statements – it prevents me from bashing my head against the wall thinking that something tested for false instead of true.

Of course, nothing beats seeing code that you yourself wrote years ago being a) horrible and b) working, but not worth the cost to rewrite, even if the cost is only a day to rewrite (and probably 2-3 days to re-test everything touched by the change).

Personally, I prefer Blurr’s version, although I’d point out that
If chk1.value then cPrice = cPrice + 7
If chk2.value then cPrice = cPrice + 3
uses one less operation per line, as chk1.value is just as serviceable a Boolean as the result of the comparison.
But then I cut my teeth on IBM Assembler, back when every operation and byte of code or storage was important.

Once I was on a team working on a consulting project and was going through some VB code written by someone from a company which will remain unnamed but which rhymes with “debenture”.

In it was a Leap Year checking routine. That is, given a particular year, this code would tell you whether it was a leap year or not.

Most people would, if trying to decide if a year was a leap year or not, see if the year divides evenly by 4. (Yes, there are the every-400-year rules, but that’s really unnecessary for this purpose.)

The code I came across was, instead, this (changed to pseudocode for the benefit of the general readership):
IF YEAR = 1984 THEN LEAP_YEAR = TRUE
ELSE IF YEAR = 1988 THEN LEAP_YEAR = TRUE
ELSE IF YEAR = 1992 THEN LEAP_YEAR = TRUE
ELSE IF YEAR = 1996 THEN LEAP_YEAR = TRUE
ELSE IF YEAR = 2000 THEN LEAP_YEAR = TRUE
ELSE LEAP_YEAR = FALSE

What mystified me is that this is actually *more* work than doing it the right way.

Sartorius: I actually read in a VB magazine many years ago that the best way to check whether a year was a leap year was along the lines of:
date = “Feb 30th, ” + year_to_test
return is_valid_date( date )

(It’s been a long time since I’ve used VB, so the syntax and function names are probably off, but you get the idea. :)

Also, I think that the code is fine. At least until 2004…

(i.e. It’s no, unless it divides evenly by 4 in which case yes, unless it divides evenly by 100 in which case no, unless it divides evenly by 400 in which case yes. So 2000, being divisible by all of the above, would end up as a “Yes”.)

The use of a trinary in Rob’s evaluation of Blurr’s example is incomplete as it implies only a 2 state solution dependent on the state of chk1. As there are actually 3 output states based on the 3 inputs from chk1 and chk2 the use of the trinary as above could lead to confusion when reading the code as to programmer intent and eventually a bug when someone changes the code downstream.

Blurr’s solution is elegant in that it takes into account both inputs and merges them into a single output of one of three states. The abstract code block to generally describe this would be:

To make the example even more abstract and generally applicable, imagine that output.value is a string representing bits that is initialized to the empty string and inputN.state.ToBitString() will be a 1 character string of either 1 (on) or 0 (off). Adding to the string will force a concatenation on the right (We define Empty String + Non Empty String = Non Empty String):

This will give you an N length bit string with state information ordered as:

Input1, Input2, Input3, …, InputN.

Then all you need to do is parse the output.value to determine which output to use. Regular expressions make this both a lot of fun, a learning experience, and an exercise in frustrating the less educated reader (Always comment your RegEx’s because people *will* hunt you down and murder you). The best expression here would be to just draw a state diagram and simplify as much as you can.

As an addendum, the memory usage of my solution would be N Bytes which represent an N bit string (So we’re wasting N*8 bytes!!!). While not optimal, I find strings to be happy little data types that I can do horrible, horrible things to. The preferred solution would be to actual manipulate a true bit string (Or bit array, if you will), but bitwise operators give me the hives.

If chk1.value = 1 then cPrice = cPrice + 7
If chk2.value = 1 then cPrice = cPrice + 3
(cPrice would automatically begin at zero, so there is no need for anything else)

Efficiency is measured with effectiveness in mind, and effectiveness is dependent on the requirements. While this snippet is defintely capable of achieving the output requirements, it may not be good code from a maintenance and risk-avoidance perspective.

without the context available to me, it’s hard to see what other possible non-functional requirements are evident, but your assumption that cPrice would automatically be zero at the start is risky; depending on programming, variables may not be declared or initiatized properly, or if this is a function called by other code, the variable cPrice may not be zero at the start of this code snippet. Hence, it is extremely unsafe — given my limited understanding — to assume it would always start as zero.

Another issue is long-term expandibility. As noted elsewhere, there’s yet another assumption that chk1 and chk2 are two check-boxes and would stay that way indefinitely. It is particularly galling to make code changes to your snippet if the business requirements later change to such that when chk1 = 3 and chk2 = 2, the output should be 15.

Note that I’m not saying your teacher’s code snippet is superior; there are weaknesses in the code snippet too. The thing is, what you are attempting to achieve is efficiency in arriving at the pre-determined results, while what he is attempting to teach is what I term “defensive programming”, code that are narrowly focussed and do absolutely nothing when presented with unintentional data/ situations. His code snippet is inefficient from your perspective, but that’s because he has got more agenda up his sleeves.

Once you get a job as a developer, you rarely have time to consider programming approaches and be nice and tidy — often, you would be under a tight deadline and force to code in the fastest, quickest way you can. That’s not necessarily producing the fastest, quickest code… just producing code in the shortest amount of time. In these situations, coding habits take more precedence over good practices, so how you tend to code in calmer times tend to be what makes it into production.

What your teacher is likely trying to do is to drill into your class a particular coding style that would ensure at least somewhat safe code (developed in a rush) that goes into production. It’s terribly inefficient from a purely computer-science perspective, but it’s not too bad when you consider that such codes have to be coded quickly and survive in production for perhaps decades — often outlasting the original programmer in that organization!

“His code snippet is inefficient from your perspective, but that’s because he has got more agenda up his sleeves.”
If I held my programming teacher in a higher regard, I might have accepted this statement. As it is, he speaks in ebonics, can’t spell, and writes multiple choice questions with multiple correct answers(And counts only one of those correct answers correct until you argue with him). He might be a genius, but he doesn’t show it.

“It is particularly galling to make code changes to your snippet if the business requirements later change to such that when chk1 = 3 and chk2 = 2, the output should be 15.”
It would be, except for the fact that in the case of checkboxes, .value = 0 means unchecked, .value = 1 means checked, and .value =2 means disabled.

The variable is local and is initialized just before using that code. And even if it wasn’t zero, there would proabably be a good reason for it. The lines of code I used are, in my opinion, much more expandable(Technical term?). If I wanted to start off with a $20 item and add accessories (the checkboxes), I would simply set cPrice to equal 20 before I ran those two lines. I’m used to OO, and we haven’t learned that in VB, so I’m trying to make my code close to being OO as possible.

Not trying to get into an argument on the Internet, particularly on somebody else’s site. Just some food for thought.

If I held my programming teacher in a higher regard, I might have accepted this statement. As it is, he speaks in ebonics, can’t spell, and writes multiple choice questions with multiple correct answers(And counts only one of those correct answers correct until you argue with him). He might be a genius, but he doesn’t show it.

Hmm. That he’s a bad teacher is not too surprising — a lot of teachers are not good at transferring knowledge, from my experience. But that doesn’t mean he doesn’t have a point.

It would be, except for the fact that in the case of checkboxes, .value = 0 means unchecked, .value = 1 means checked, and .value =2 means disabled.

And if someone ask to change the checkboxes to a select list? How much extra work would your code need vs. his code to comply with the changes?

That’s really what I wanted to highlight. Your code is efficient — for that one particular scenario. But if the scenario changes on you, the effort to make change can be hefty — especially the verification effort.

You can say that for the code suits that particular scenario, which you would be correct to say that. But in the commerical market, it’s not just whether a piece of software meets a particular requirement, but also how flexible the software is when the requirement (inevitably) changes.

The variable is local and is initialized just before using that code. And even if it wasn’t zero, there would proabably be a good reason for it.

Yes, the variable probably is controlled — if you coded the rest of the code.

Thing is, when you start working as part of a software team, it’s no longer just you coding alone, so you can’t be sure whether there’s a good reason for things not being done the way you expect it. Sometimes it’s sheer carelessness. Sometimes it’s sheer stupidity.

The lines of code I used are, in my opinion, much more expandable(Technical term?). If I wanted to start off with a $20 item and add accessories (the checkboxes), I would simply set cPrice to equal 20 before I ran those two lines. I’m used to OO, and we haven’t learned that in VB, so I’m trying to make my code close to being OO as possible.

Ah, so it’s an online store application? If that’s the case, neither code snippet is particular healthy; in order to expand beyond the one item (i.e., you sell more than one item in the store), you need to duplicate the code again and again and keep changing the base price and accessory prices. It’s probably too early in the course to teach about this though.

There’s also another aspect: Debugging. If, for some weird reason, the application returns a cPrice of “21”, you sort of have to figure out where this happened. Under your teacher’s code, you can pretty much eliminate this particular cPrice as a possible candidate (because it’s a hard set and never varies outside of 0, 3, 7 and 10), whereas your snippet might be contributing to it.

The speed at which you narrow down your bug search area is very important when it is a real-time system and users are screaming down the phone line at you. At that point in time, having “poor” code like your teacher’s might actually be helpful (though I think it’s not the greatest code snippet in the world either..).

For the process which is being represented (adding up the price of various items), I find Blurr’s solution significantly more readable. In particular, since it more closely resembles how prices are added in concept (store clerks don’t make an aggregate of every item you’re buying and then look it up on a huge table), the purpose is more easily determined from the code.

Secondly, it’s relatively scalable – adding a new checkbox adds a constant amount of code, whereas with the teacher’s version, it would add an exponential amount of code.

Third, it doesn’t try to build an orbital laser cannon to swat a fly. A hyper-expandable version would do something like load a database file with the prices, and then iterate through the checkboxes in the GUI container, assigning a price to each one from the database, then sum or otherwise combine the prices, as directed from a configuration file. Then it would format the price as defined in the config file, before outputting it.

And for many applications, this would be massive overkill, would make the code harder to maintain, and would still have to be redesigned anyway, when an unanticipated change was required.

I know my code’s pretty bad, probably a mash if a couple languages, but you get the idea. It’s more or less infinitely scalable both for new items and multiple sales. I’m thinking use a text file to load the price list in cents, rather than mess around with floating point numbers – just format the output to put the decimal point in.

You’d just build the array of prices when you started the program at opening, so there’d be very little overhead in terms of CPU cycles – order N after initialisation.

I think there’s just no good answer to the question. Like Null said, no point building an Orbital Laser to kill a fly. (Unless you like collateral damage, in which case an Orbital Laser is probably underkill…)

The impt thing is to recognize that there is good coding practices for some situations that are absolutely bad for others, and that in reality, it’s really a balancing act between coding/ programmatic efficiency, and other factors such as long-term flexibility and maintainability.

First, Blurr, put the flame torch down and back away slowly. Ebonics is a valid dialect of English, and its usage indicates identification with a certain ‘black/urban/southern’ culture. Your including of this in your series of complaints about your teacher raises my hackles, as a linguistic geek. If the point is that you can’t understand him, you should simply say that — preferably to him.

As to “uncommented code”, I knew the mad game developer who never commented code. He got other people to comment his code for him, because in his words “if you make me comment it, I’m using other games’ debug messages”

Self-same person’s response to needing to maintain his code was to write a self-modifying compiler to deal with the self-modifying code (and if the code ever got into fatal error mode, just reboot from earlier version).

Same person gets paid by the gov’t to insert bugs into his code, so that the Q&A folks can go fix them [in all fairness, no one is supposed to be able to write 50,000 lines of code without three errors in it].

And if you ask him to comment his code, you’re likely to get a scientific article (that you will still find indecipherable — as good as he is at communication, his brain works fast).

If you ever meet this guy, I suggest you run, and run quickly.

I don’t mind badmouthing the last guy — his social skills were poor enough that I can get away with it, and I don’t mean much by it.

But in my last job — my boss was the last coder. So… if he wanted to do the same task three different ways (which made it impossible for the non-programmers to double check his timing), I was stuck explaining what he had done.

If you’re writing C or C++, a tool like “lint” can supplement the compiler and help catch suspicious errors and still keep your code readable.

I’m not saying that the defines are making it unreadable, but they do obscure the code, and make things harder to debug when someone else is trying to read it later on. Even if the defines make perfect sense, they’ll slow down those who are unfamiliar with them.

I think it depends on what you mean by “speaks ebonics”. If he uses phrases that obfuscate his meaning, then he’s a rotten teacher. If I hired someone from Germany and he threw German words into his speech willy-nilly so that his English-speaking students couldn’t decipher his lessons, my response wouldn’t be to tell the students to be more culturally open-minded. The bad spelling and poor communications skills reinforces my perception of him as a guy who can’t communicate.

It’s not a matter of “is Ebonics valid”. It’s a matter of “Does his speaking impair his ability to impart knowledge?”

I wouldn’t call it a “stupid” define trick. Something like that would have to be part of your in-house coding specifications / standards. It has metrit, mostly for avoiding the evil “assignment instead of comparison” problem for which the double equals is notorious.

I kind of like the look of the comparison statements when done this way. Even after reading code for all these years, I find the ((a AND b) OR (NOT c)) to be a lot more readable. Still – preferences vary, and it’s not something I’d introduce lightly, because of the confusion it would cause to the un-initiated.

Oh, I’ve had the bad teachers, Shamus. You’re just echoing the point I was trying to make — it being about communication and not what variant of the english language someone was speaking.

But the person who writes entirely in Chinese on the blackboard (and speaks unintelligibly), is either not teaching the class you signed up for, or is probably not the problem (the problem being that anyone allowed said teacher to have a classroom with non-Chinese students).

I’ve also had the chronic mumblers (not confident enough in their English to speak up), and that’s a far trickier question.

Also, Shamus, if his speech impairs his ability to impart knowledge — is it your turn to change, or his? If you were in England, and having substantial trouble understanding a teacher, you’d probably think that it is your responsibility to change. However, if the English Teacher was Stateside, you’d think it would be his responsibility.

And then, there’s the question of whether it is intentional, ‘partially corrected’ or not. The professor may be unaware of the problem — or trying as hard as he can to correct it, and may be able to pass along some grammar standards to aid a confused student(if not, google is your friend).

There’s this urban myth about Nixon. When he left, he put two envelopes on the desk in the Oval Office, with instructions to open one envelope every time the new guy got in REAL trouble.

Well, there was a big fight about education and he was doing pretty badly in the polls, so he opened an envelope. It told him to blame everything on Nixon’s legacy, declare his party the party which would fix it and do his best.

That worked pretty well, so when the big corruption scandal hit, he opened the next envelope. The letter inside was much smaller, in fact it only consisted of three words: “Write two envelopes”

So I was trying to avoid commenting on the code thing… but it’s seriously biased. The ‘seasoned professional’ thing includes part of a C++ library. It is not bad practice to make use of libraries and high-level languages.

“One of the harder lessons that I’ve learned is that programming for values that shouldn’t make it into internal functions increases the number of bugs I have to deal with. It handles the wrong input gracefully, thus hiding the bug and making it harder to track down. If it instead chokes and throws a stack dump, I know about the problem where I would have been unaware before.”

Alas, it also means that the stack dump might occur in production…in which case the users’ fragile minds snap and they go screaming to management…which then sends the developer anything from a sarky e-mail to a pink slip.

I hold with the Gneech, with this caveat: it is not true that, if all you have is a hammer, everything will look like a nail; even if given a full toolbox, some people will insist on hammering screws in with a wrench.

At least in all the codebases I work with, the assembly code gets good comments.

I don’t mind if the rest of the code isn’t terribly well commented (though i live for white papers) — but I don’t read assembly, and even if I did, spaghetti code is a lot more annoying in assembly (whereas one or two gotos in C++ get redflagged, and you search the entire codebase for what is jumping there).