Warning Signs Of Bad Programming

That slight tension of the brows which reminds you that mental effort is required to understand the program.

Unless of course it was written by a programmer who understands more patterns than you do. It is possible to write a very clean, simple, beautiful program that another-probably less experienced-programmer would find baffling. For example, if you have no exposure to patterns using a FunctorObject, code that uses them can be pretty baffling until you realize what's going on.

By the way, 'GoTo earlier statement (to create loop)' is what may confuse; 'GoTo later statement (within scope!)' is relatively harmless.

I disagree - GoTo earlier statement to create a loop is ok, going forward is bad ... and any use of GoTo is usually a sign that something is wrong. -- NissimHadar

But GoTo isn't usually needed for loops, whereas those who don't use it to go forward usually achieve much the same thing with nested IF statements instead, and end up with something less easy to understand than it would have been if GoTo had been used. (I'm referring to what tends to confuse rather than suggesting that GoTo should be used.)

For me, personally, overly-long methods together with badly-named variables and methods are the number one sign. For if the person doesn't even bother or know how to refactor his methods, the same will probably be true for his classes and his program. At least, that's what I guess. -- FalkBruegmann

In a VisualBasic environment...

In addition to the above, lots of Global Variables, arrays when collections would do and the performance is not that tight, dense logic in form events, or in forms at all. Dates held and manipulated as strings. -- AndyMorris

Warning signs? Who needs warning signs? I've yet to walk into a project that didn't have severe problems with CodeQuality?!!!

Well, I guess it's all those puzzled looks and statements of "we don't really know why our software is so unreliable, hard to support and hard to maintain; we write lots of code, but it always seems so difficult." -- anon

Unnecessary complexity. If, during a review, you can't understand someone else's implementation, then typically neither can they. -- ArnoldLayne

In a language like perl or C,

Reference fields with offsets instead of names (foo[0] instead of foo->bar)
Use arrays instead of structs or classes
Use too-cryptic variable names (sc instead of sampleCount)
No white space (long lines, no vertical space between functions, no indents)
Inconsistent use of scoping, global references scattered randomly through code

Whether methods or functions in non-object-oriented C or similar languages, the huge size often results from a TotalNeglectOfRefactoring, which I believe is the most common cause of the truly wretched code messes so many of us must work with. --AnonymousDonor

I have found that CopyAndPasteProgramming is very often to blame, as well. From BadProgrammer:

One of the best coders I ever knew was a notably BadProgrammer. We've all SeenThisPattern: he can write code faster than he can design it, so when he needs a the same operation done in fourteen different places, rather than write one function to handle all fourteen cases, he writes fourteen slightly different functions scattered about the code. Multiply this by the number of times he's done this and by the number of changes that have to be applied to each similar function, and you have a maintenance nightmare that can bring a project to it's knees.

I'm not buying it; I can normally design the function in much less time than it would take me to find the 14 insertion points and hit control-V - much less make and verify the minor modifications (of course, the tests are the real verification regardless, but I always want my tests to pass the first time, and I'm much more confident calling a function than modifying copy-and-pasted code). Seriously, CopyAndPasteProgramming slows you down in most cases, even initially.

I teach students how to refactor the duplication out, yes, but I also emphasize the importance of avoiding it (at least in trivial cases) in the first place - you should only have to refactor it out of someone else's code ;)

-- KarlKnechtel

Even in AssemblyLanguage, where I spend much of my time, there are warning signs. Some of them are not unique to Assembly:

Inconsistent CaPitALIzatION
Inconsistent use of whitespace
Poor or no commenting
Poor or no separation of code into routines
Poor or no organization of variables

In essence, code that looks bad probably doesn't run well either. -- EdBeroset

See also: CodeSmells

How is this page different from CodeSmells?

Code starts to smell after a while even when doing "good" programming, unless you refactor your code. Bad programming makes it stink immediately.

So this page should be called CodeStenches perhaps?

Even wonderfully prepared gourmet meals begin to stink and become unappetizing after you've left them on a countertop for a week. Poorly prepared meals with low quality ingredients are unappealing immediately.

UML diagrams

UmlDiagrams that are considered design. Any real engineer knows the difference between the plan and the design. Real designs are such that Boeing can estimate the mass of one of their planes within a couple hundred kilograms from the design alone. By weighing the design doc and taking that as the estimate, right? ;) In software, TheSourceCodeIsTheDesign. UML is the plan, and as such, should not take more than a couple weeks to write, if at all - we do iterative design in software, after all. Really, they should rarely exist because UML diagrams communicate very little. Not only are UML diagrams C++ dependent (C++ in pictures!), but they are chiefly used to communicate class relationships. Class relationships are apparent from the codebase. Not many people write interaction diagrams.

: Yay to this, with bells on. UML as I've seen it used is a great way to cast everything in cast-iron and stop any refactoring ('because it takes too long to update the model'). I think it's a good tool to communicate and document the global idea... but design???. (cf UmlConsideredHarmful) Minor caveat: I suspect UML might be usable with other languages than C++, but this is only my personal experience :-) -- RomanStawski?

Actually, this gives me an idea. Next time I get trapped in a useless "design" meeting, I'm going to ask where the interaction diagram is. That should delay the wankers for another two months. ;) -- SunirShah

"Any real engineer" knows that various design resolutions are needed for non-trivial products. For a software application, there is a resolution where the whole application is one box in an enterprise services deployment diagram. Then there is a worms-eye-view of the product in the code. For resolutions in between these two, UML models are effective and economical. For non-trivial products, there are a lot of risks and design issues you want handled well above the code. Modeling is how you do that. If UML is getting in the way of code refactoring, you aren't using it properly. If you are trying to do high level design in code, you are using the wrong tool, and that is Bad Programming. --MarcThibault

No UnitTests. -- ChanningWalton

Accessing properties of an object from outside the object. Big no-no, make methods for returning the property instead.

Funny. Recently I had the epiphany that EncapsulationIsaWasteOfTime. -- SunirShah

They've rewritten the same thing 5 times in the last year, from the ground up, "because the new changes the customer wanted were too big to make the old stuff work".

: If they had been using XP, they'd have rewritten it more than that. It seems that the principal difference is a principle difference.

Having to alter 40 classes in a half-dozen packages in order to add what should have been a simple subclass. The lead programmer on a project explaining to management that the hundreds of NullPointerExceptions being generated daily by an application were not a problem because "it just means that the software expects something to be there that isn't."

90% of the business logic is in database triggers.

Worse - 50% of the business logic is in database triggers.

I would suggest that the warning signs of bad programming are not important or enlightening; all programs will show signs of bad programming. The key issue is whether something is being done to address the bad programming. Face it, sooner or later we all are going to write something that we wished we didn't at some later time. If a project has a legitimate, above ground refactoring process, the sections of bad programming will get uncovered and fixed. If not, the project is just hoping everyone writes perfect code, without fail.

We have to be able to spot bad code, but we must use this power for good and not for evil, to improve the system rather than to blame someone for wrecking it. Improving the code and the process that produces it is constructive; blaming people is destructive. So it's better to identify CodeSmells than BadProgrammers.

Nonetheless, we often put too much focus on code and not enough on process. You can rearchitect and rebuild all you want; but if you keep doing what you've always done, you'll keep getting what you've always gotten. Without practices like UnitTest and RefactorMercilessly, your designs will continue to degenerate, no matter how much thought you put into architecture.