tags:

views:

3270

answers:

83

Maybe infuriate is not the politically correct term, but what kind of code would qualify for a genuine face palm?

Addendum: For me, it's the misuse of technology. The group of people who develop .NET like classic asp apps are very likely the same group of people who use recursion for simple iteration, standard array where linked list is blatantly the answer, massive number of individual variables in combination with if-statements for hash tables, functions for properties, validating input forms with only javascript, placing important naked-eye readable information in cookie,and on and on and on....

+8  A: 

Right now? Large amounts of CPP files with only 1 H file for the entire project.

Paul Nathan
Agreed. I recently started looking at some old cpp code with a header file took me about an hour to look through. It could have easily been split up into four or five files to make more sense. The bad part was, it was code that I wrote.
ZCHudson
oh that sent a shudder through me.
Setori
A: 

Code which is lacking comments of complex algorithms/methods, poorly formatted or has bad variable names.

Kevin
+39  A: 

Poorly-formatted code. There's just no excuse for that.

All code gets crufty, complex, messy and obscure. But a basic sense of taste is sufficient to keep the code well formatted.

ddaa
he's right tho, it's not just loop indents and the like, it's separating you code within functions into logical blocks space spaced and commented if appropriate - that can't be done automatically - you need to understand what the code is doing
Cruachan
This really bugs me too, because with all the prettyprinters and IDE's these days, someone almost has to put in physical effort to deliberately misformat code - yet it still appears everywhere :-(
Orion Edwards
Inexcusable, even without a prettyprinter, just indicates that they didn't care
seanb
@Bill K: The problem comes when the section of code in question have generated tens of bugs. If you ctrl-shift-F it, you will be winning the raffle of the tiger ( does that saying exists out of where I live ) I mean, the VCS will give you the paternity of such code. :S :S
OscarRyz
"paternity of code"? it doesn't have to be like that... see shared code ownership (http://martinfowler.com/bliki/CodeOwnership.html)
Mauricio Scheffer
and it could be down to more than just "looking nice": http://www.levelofindirection.com/journal/2009/9/23/code-formatting-in-c-part-one.html
Phil Nash
+24  A: 

I really hate "Smart" algorithms without any comment. They cost enormous amount of time to comprehend, and most of the time, the "smart" factor is not that high.

Gamecat
you mean "cryptic", not "smart"
Jimmy
I have yet to find one in my project. Sigh.
Haoest
Actually "Smart" isn't bad a description of the problem. These days any time someone says the word "Elegant" I cringe.
Bill K
I've redefined "elegant" to mean "the most obvious way to do something". Spread the word. Hopefully it will catch on.
Bill the Lizard
Lol, in remarks: "A" == !A ;-)
Gamecat
Commented code FTW
Jonta
+1  A: 

Going overboard with "normalization" so that the actual problem domain is hidden behind cryptic codes.

For example:

lblStateProvince.Text = myObject.StateProvince; // Good
lblStateProvince.Text = Utility.LookupCode(myObject.StateProvinceCode); // Bad
Larsenal
+97  A: 

Finding essentially the same code cut-n-pasted in dozens of locations throughout a project, sometimes with one or two subtle changes.

Adam Liss
I love finding these. Refactoring them can be really fun, and can pull out some great OO patterns in your code that you wouldn't have thought existed.
Bill K
Just fixed one of those today, in fact.
Electrons_Ahoy
I love fixing this bits of code. My favorite type of refactoring
Nathan W
I've heard this referred to as "clipboard inheritance". 8)
Carl
Wish I could +1 for clipboard inheritance - you *know* we'll all be using that term on Monday morning! Thanks, Carl!
Adam Liss
Been fixing this for 6 weeks now. Sucks.
"Clipboard Inheritance" is now a stock phrase in my refactoring comments. +1 to you sir!
Electrons_Ahoy
I worked on an old classic ASP site once where the database connection string--complete with password--was repeated for every SQL query on every page of the site. I wept.
Nicholas Piasecki
+1 for Clipboard Inheritance
Rakesh Pai
At one of my old jobs, I used to say we had 100% code reuse: Control-C, Control-V.
Kyralessa
+30  A: 

Regions.

I hate opening a file just to see 50 collapsed regions, each of which containing another 50 collapsed regions. It's bad enough I have to dig through the object hierarchy, but now I have to dig through the region hierarchy the developer came up with!

Kevin Pang
Use ctrl + m + l to expand all if you don't like them rolled up.
DancesWithBamboo
Yes, that is always an option and you can always ignore the #Region ""and #End Region lines. However, modifying that file is still difficult since any changes you make have to fall into the correct region. Otherwise the code becomes even messier. :-)
Kevin Pang
I actually like regions if it's well defined. If your company has a standard policy of collapsing class fields, properties, event handlers, ETC, it can be very convenient and time-saving. But like all other tools, it can be abused.
Haoest
Amen. Regions are even worse when you think about how much time programmers spend on them, when they at best offer a tiny bit of utility (I'm being polite).
MusiGenesis
Regions would not be necessary if VS' implementation of collapsing and expanding methods was better.
Ed Swangren
Amen! to that. There should be a global IDE option to completly ignore Regions. They are the work of Beelzebub! :)
Mitch Wheat
Frankly I see not why you would have to use a language feature for the presentation of code. THis is context bleed, much like putting the styling in HTML. Code styling belongs to the IDE, I have enough of programming the system I cannot be bothered with programming the IDE through the code.
Newtopian
+1: Sweet Jebus I thought I was the only one! I am not alone!
Binary Worrier
Yes they are awful, especially because everybody uses them to their liking. More times then not, if you got really lots of region, it suggests lower cohesion within that class, so refactor that code man!
Sander Versluys
+48  A: 

Inane comments. For example:

// increment i
i++;

A better comment:

// i is off by one after the above, so adjust it
i++;

What the code is doing should already be obvious to any competent programmer who's familiar with the language. Comments are for explaining why it's doing it.

Sherm Pendley
But still, inane comments are easy to remove. Copy-paste code with subtle changes are not.
JesperE
Naming of the methods and variables should be so obvious that knowing the reason why shouldn't need to be explained in comments.
JtR
I agree, but I think your second example is flawed. It actually says *why* the increase happens.
DR
This is a comment :P
Cameron MacFarland
This is not a pipe.
Carl Manaster
@Cameron MacFarland - That'd be a perfectly acceptable comment... If you were writing a tutorial for some programming language and wanted to tell the reader to the comment syntax.
luiscubal
+14  A: 

Inconsistency.

You can't trust anything in inconsistent code. Even if the previous developer used conventions and techniques that make me wince in pain, if he was consistent about it I can at least learn to identify with my torturer. Inconsistent developers deny me even that faint comfort.

You can't really trust anything in consistent code either, mind you. But it's one thing to be able read code like a programmer, and another thing entirely to have to read it like a compiler or interpreter.

Robert Rossney
+14  A: 
public class Programm{
     public static void main(String[] args){
     if(bla bla bla bla)
     ...
     ...  

     ...
     //few thousand line later, the record is 60k
     }
}

arg...

muuuust kiiiilll


Another thing that kills me is finding new patterns...

ever seen an Abstract Singleton anybody !!!

Newtopian
Did you really see a 60k lines 'if' block?!?! That is calling for the Angel of Death
Federico Ramponi
there were a few thousand ifs in all, sometimes 20 level deep with plenty of goto (it was in c despite the java feel of the example above)
Newtopian
New patters are great if they make sense. Any method over a couple screens needs to be refactored. I kind of wish compilers would cut you off after a hundred lines or so and give you an error.
Bill K
spaghetti code in a thin OO wrapper. yup, seen that, it sucks bigtime. how do i add more than one up point to this answer?
DarenW
@Bill... Totally agree with you, nothing I like more than a new clever way to tackle a problem, but sometimes they can get a little too creative. Mix a bit of the Onion (my favorite antipattern) with that and you get masochist pure joy
Newtopian
@DarenW I did it for you
Haoest
You don't want to cut off after a couple hundred lines--there are a *FEW* cases where it's reasonable. All of them at their heart involve a large switch statement or equivalent. (I've hit it with a massive if..then..else.if..then that was really a switch but the key was a string and Delphi doesn't do strings in case statements.)
Loren Pechtel
my biggest beef here is not so much that the if statement was big but that the whole program was a main function. No class, no struct no sub function nothing... start the main... start coding and close main.. that's it. craazy
Newtopian
+35  A: 

Kilometric methods. I've found methods of about 100 to 400 lines of code, with 5 - 10 nested if, for, while, do..whiles and tens of variables with names as

tmp

var

var1

a

i

x

y

z

I really hate those methods!!!

OscarRyz
Doubly so, when that 'tmp' is used and reused. And reused some more, just for good measure. Also, tmp2, tmp3, tmpn.
Matthew Scharley
Oscar Reyes: What's a kilometric method?
blizpasta
The kind of method that if printed will consume many trees. :) AKA 800 lines or so.
OscarRyz
A method which is measured in kilometres, of course!
JesperE
@monoxide: I have seen such a method where that 'tmp' variable was not just used and reused, but redefined in an inner scope and used again. Trying to find out what the code was doing, just by looking at it was impossible.
Rohit
Usally I take a deep breath and say.. Ok, I'm going to wear my "Refactorer hat" and fix it. By the 3rd hour of not getting anything done, and not have a clue where to start, I go to the line 756 and add another cryptic if then else, test and verify the bug is gone and close the editor. :( :(
OscarRyz
The largest function (it was plain C) I ever saw was 9000 lines.
Gamecat
are you saying methods measured in miles are ok :P
hasen j
Complete agree with Oscar Reyes .. this is what 6 years of EDA tool development has made me ...
Vardhan Varma
+1  A: 

Code where someone undisciplined decided that they had to do things their way (usually because it is the ONLY way according to them) instead of sticking with the convention that the code already had going. Then you have to sift through all the different styles.

DancesWithBamboo
+14  A: 

I just got done maintaining some database code and the guy just did not understand the concept of working with sets of data. EVERYTHING was single row actions, looping through using cursors. There were no multi-row updates, just a cursors looping through updating each and every row.... That was just a recent example.

So in a nutshell, developers not understanding the type of system they are working with and writing appropriate code.

Brian Schmitt
I hate finding code like that.
Mitch Wheat
Exactly. That classifies as misuse of technology.
Haoest
+4  A: 

The reviewer shall also need to keep in mind the old adage: To every programmer, other programmers' code is s***t ;)

  1. Lack of automated Unit Tests...
  2. Not leaveraging language features (if you're using VS.Net 2008, you better use Automatic properties, etc.)
  3. Mind numbing readability ;)

    if(x ==1) { y = 5; } else { y = 10; }

  4. Use of hungarian in C# code
Vyas Bharghava
I had a coworker who once used hungarian notation in cold fusion code. All his strings were like: lpszFirstName.
jonnii
Vyas Bharghava
Use of the type-oriented "hungarian" in *any* code.
Software Monkey
That's right, SM... I agree [I remember having taken this to extreme as I used to have spaces in my column names in MS-Access like "Customer Name"... I used to say customerRecords![Customer Name] in VB to access the same..Eons ago! (sigh)]...I cry when I see names like "Cust_Name" and "tblCustomers"
Vyas Bharghava
+10  A: 

Slope code:

if(...){
    if(...){
        if(...){
            if(...){
            }
        }
    }
}

And people who over used singletons. WHY!? I know you read a book on design patterns, but that doesn't mean you have to use every single one you find.

jonnii
Singletons are bordering on anti-pattern. Not quite, but if you find yourself using them a lot you might want to look into IoC and DI systems like Spring.
Bill K
This is also known as arrow code: http://www.codinghorror.com/blog/archives/000486.html
Cristián Romo
People use singletons because singletons are basically procedural programming, and easy to grok. Tends to be the hallmark of a junior developer, tons of "*Manager", "*Helper", and "*Utility" classes
Matt Briggs
Well it also depends on whether or not the language has short circuit evaluation. If the language (*cough* VB *cough*) doesn't, then your code will perform better if you don't evaluate multiple conditions in an if. You either need to do each condition, or throw it into a Select Case.
Oorang
A: 

Kind of nit-picky, but when everything in an OO system is get and set with no comments, I end up needing to bug a co-worker to find out what the heck is going on, which I think is wasteful.

I also hate useless Middle Men that do nothing except forward messages to another class. The only reason these classes exist is because some architect decided it was required. Gah.

moffdub
Middle-man layers are sometimes useful in maintaining code
Paul Nathan
Bill K
Vilon, I understand Middle Men are useful to, for instance, Hide Delegate. But I've seen cases at my workplace where *every* method in class A calls a near-identically-named method in class B. In that case, I have class B implement the same interface that class A does, and delete class A.
moffdub
Bill, I totally agree. I think a big reason people, including myself up until fairly recently, don't get it is because methods in popular OO languages (read: Java, C++) look way too much like functions in procedural languages. Merely reading about Smalltalk made it all click for me.
moffdub
It's very unfortunate that Sun decided to implement beans the way they did. Beans should be pure data transfer constructs with no code associated with them at all (if all they are are setters and getters). If you want to add code, wrap a class around it...
Bill K
+23  A: 

Reinventing the Wheel

I just hate it when I come over some internal implementation of functionality which is in fact part of the platform.
And no one has a real clue about why it was implemented that way, and not used the existing platform implementation.
Especially when the internal implementation is a sub set of what is provided by the platform.

Alex Shnayder
This gets even worse when other programmers take the reinvented wheel and reuse it without themselves knowing it.
JesperE
Even worse than that is reinventing the _square_ wheel.
CesarB
Early (very early) in my career I rewrote sort() because I needed it to sort backwards. Who knew you could just send in a comparison function! :-)
SquareCog
I have been forced to reinvent things in current JDK versions because we have to target old JVMs - I only recently got to begin targeting Java 1.3 in my work; and I am having to push mighty hard to be allowed 1.4... what I wouldn't give for 1.6!
Software Monkey
Even better on JVMs is when you need to support more than one at once, you have to rewrite the function in 1.3 thats introduced in 1.4, but then do you use both depending on which JVM your running in?
Greg Domjan
I once wrote a DateDiff function for VB6, little did I know there already was one that took into account leap years. arghhh.
Neil N
+14  A: 

Excessive "wiring", where some idiot developer got so jazzed about eliminating "dependencies" that none of the classes actually refer to each other, and everything is hooked together in an enormous XML file.

Reading the code, you can't actually tell how anything works anymore, because the ordinary flow of logic is obfuscated until runtime, when all the functionality is magically woven together by some over-architected framework.

Yeah, sure, you can ALT-TAB back and forth between the code and the XML and try to remember how all the classes are wired together.

But first, you should walk down the hall and punch that jackass in the teeth.


ON EDIT:

Incidentally, I'm familiar with the DI and IoC patterns, and I'm perfectly happy to admit that they're occasionally useful. In fact, if I may be so bold as to quote Martin Fowler:

Inversion of control is a common feature of frameworks, but it's something that comes at a price. It tends to be hard to understand and leads to problems when you are trying to debug. So on the whole I prefer to avoid it unless I need it. This isn't to say it's a bad thing, just that I think it needs to justify itself over the more straightforward alternative.

http://martinfowler.com/articles/injection.html

There have been certain (rare) cases when I've happily used DI. But only under these circumstances:

  • When you actually have multiple implementations of a service, AND

  • When the selection of those services must be applied at configuration time (not at compile time or runtime)

In my experience, this combination is extremely rare. Most projects I've been involved with have exactly zero justification for the injection of any dependencies. Sometimes there's a legitimate need, and yeah, in those cases DI is a lifesaver, because there's really no other way to do it.

But I can't tell you how often I've worked on a project with hundreds of service interfaces, each of which has one class implementing that interface. And then there's a configuration file with thousands of lines of XML to connect each service interface with its implementation and to inject constructors into every single private field.

It's madness!!

benjismith
That's IoC+ Dependency Injection. It can solve some HUGE problems. Sounds like you're upset because the tooling isn't able to handle it. Try testing in the debugger so you can see what an instance actually is. What you described is fantastic code except for the tooling issue.
Bill K
I agree the tools suck since you are combining XML with Java at runtime to figure out what you're debugging, but that doesn't have to be the way. Debuggers will know the actual type, and I've been known to print the .getClass() name to see what instance is there at runtime.
Bill K
No, Bill, this has nothing to do with tooling. The thing that annoys me is the developers who think IoC should be used *everywhere*. Sure, DI can occasionally solve some of those HUGE problems you refer to. But when mis-applied (more often than not) it CREATES huge problems.
benjismith
Quite frankly IoC is interesting because it is easier to do mocking to test code. However, you got a point - it sure is harder to read code developed this way - I use multiple monitors and notepad+ to use the XML as a reference (1st monitor) code in the IDE(2nd monitor) and interface (3rd monitor)
rshimoda
Limbic System
I have upvoted this because it's interesting, thought-provoking and well-argued. Not sure if it's right.
Anthony
+25  A: 

when they lie in the comments

DaveJustDave
waiting to jump out at you?
Alistair
I like Steve McConnell's quote from Code Complete: "If the code and comment differ, then they are both probably wrong"
Mitch Wheat
That quote is a very good approximation of reality.
JesperE
+13  A: 

Layers of code for no reason. Why have a business logic layer if you never use it? Ever.

In the last 3 projects I've worked on, every function in the business logic layer simply passed data through it. None implemented any processing or "rules" at all.

Meanwhile, I have an idea to save myself a lot of typing... :D

BoltBait
+4  A: 

Too many classes that don't do anything. On one project that I inherited, I found a pair of classes named "And.cs" and "Or.cs". The And class added " AND " to the end of a string, and the Or class added " OR ". They were used for building dynamic SQL.

MusiGenesis
I've used something like this because you could drop the class into a system and use it progmatically. For instance, a user can press the "AND" button on a GUI and it drops the "AND" appender into some collection and is later used to automate request building. I'm guessing that's not what you saw.
Bill K
No, this was code where instead of just writing something like "strDynamicSQL += ' AND ';", he would instead write something like "(new And()).Append(ref strDynamicSQL);".
MusiGenesis
And the Append() function would just take the ref'ed string and add " AND " to it.
MusiGenesis
holy crap. insane.
deadbug
You can't even imagine what Join.cs was like. But at least it didn't just tack " JOIN " on the end of a string.
MusiGenesis
+1  A: 

Any code that makes me guess, which means that I'm happy with pretty much anything I'm allowed to refactor or rewrite; and I'm fairly unhappy when there are pieces I can't rewrite.

I can only really understand a group of code by getting it all into my head. Code that's really poorly factored is very hard to get your head around. As you refactor and eliminate 2/3 to 9/10 of the code, it all becomes much more understandable--and the process is fun.

When I'm not allowed to refactor it, it can be difficult to impossible for me to work on it because I can't stand guessing. I don't just poke a variable and retest to see what happens, then if it happens to pass the test call it "Fixed" (which is what it seems like most people do).

It's not that I have anything against other people just kinda guessing and poking stuff in and seeing how it reacts, I'm just a bad guesser.

Bill K
+4  A: 

Code written by someone who hasn't taken the time to learn the language and any standard libraries. For example (and this is by someone I know), using arrays and array.resize to store a list of objects rather than std::vector or Generic.List. Not taking advantage of OO and type safety when using OO languages (someone else I know didn't use the class keyword anywhere in their C++ application, did use try/catch though).

Skizz

Skizz
Well, more than enough people use the `class` keyword because they saw it in a book and don't want to be seen as ignorant..
+8  A: 

The codebase I'm working on now was originated by someone who clearly never decided whether to use spaces or tabs for indentation - so they used both, completely inconsistently, often on the same line.

Sure, this project has just about every other anti-pattern you've ever heard of, but finding another line indented with "space-space-tab-space-tab-tab-space" is what really sets my teeth on edge.

(I finally started re-formatting every code file whenever I touched it, and I've got a chart on my office wall of which pages have been "fixed.")

Electrons_Ahoy
This usually happens to me by accident when a file is edited in different editors. Some use actual tabs, and others insert x spaces instead of tabs, where x varies. That can make things fun, as you described.
moffdub
it can be fixed relatively easily with code beautifier.
Haoest
Occurs all the time in emacs. Use this alias (in csh) to correct: alias use-emacs-to-untabify-file '\emacs --batch --eval='"'"'(progn (find-file "'"'"'\!:1'"'"'") (untabify (point-min) (point-max)) (save-buffer))'"'"
Mr.Ree
+7  A: 

Violations of abstraction.

For example, a stack is a stack. It is not an array. It is not a linked list. It is an abstract data structure that is to be accessed using the defined interface, not by directly accessing the implementation details. What bothers me is, for example, code not in the stack implementation that "knows" the stack is "really just a linked list" and accesses it as such.

Glomek
well if your abstraction didnt leak in the first place...
jk
+20  A: 

Code that was written for the compiler, rather than other programmers.

Code that was written because it's "clever", rather than just being smart.

Code that is written in such a way that you cannot easily dig into it using your toolchain.

moobaa
I wish I could vote this up 3 times! The first line says everything you need to know about GOOD programming.
Bill K
"Clever" code that is wrong!
TGnat
If you define wrong as "not the easiest possible code to read", then I agree. But Clever code is rarely the easiest possible code to read and comprehend, so the original comment stands.
Bill K
+3  A: 

Infuriate - no. Left asking why - yes.

  1. Pointers to pointers to pointers to pointers ****someVariable (yes I've worked on this code).
  2. Long methods with names like i and j for variables used everywhere not just for iteration.
  3. Long methods with variables named after people (yep rewrote that one too).
  4. Methods named after people (You haven't lived until you've seen a stack trace that says Error in Jeff).
  5. Long methods with variables that change what they mean over time. For example i is a counter, now it's the number of bytes in a file, now it's the screen width, etc...

Those were tough code reads, but they make for a good story.

Mike Daniels
Doesn't *** indicate a dynamically allocated 3-dimension array? How else would you implement that in C? Also--I occasionally put my initials in a method to remind me to change it once it's "Solid". Don't think I ever let one escape, but I've been sloppy before :)
Bill K
Two level pointers: ok. After three levels start using typedefs. Soooo much better to read!
Zan Lynx
+4  A: 

Any code that is way too complicated for the job at hand. To quote a colleague:

Don't take the space-shuttle to Kansas City: just take the bus.

(I am in St Louis, USA, 5 hours from KC. Replace cities as appropriate for your locale).

Michael Easter
No matter which locale, taking the space shuttle is still overkill.
CesarB
I disagree: sometimes one does want to go to the "International SpaceStation". e.g. I consider AOP to be a space-shuttle but it definitely has its uses in the right places.
Michael Easter
I think they're putting a night bus route to there also, soon :)
ldigas
Upvoted just for the quote. :-)
Limbic System
+2  A: 

I have inherited programs in COBOl that were completely littered with GOTO statements and variable names that started with A and when the alphabet ran out they started over with AA, AB, AC etc...

Dan Adams
They were just safe-guarding their IP with obfuscation ;)
Software Monkey
+24  A: 

Massive 4000 line files or classes, with massive if statements everywhere.

nportelli
+20  A: 

In my younger, more vengeful, days, I was once irritated by a colleague who declined to fix a trivial bug that I had reported. It was a very quick flicker of the fields in the GUI when items changed.

The bug was bothering me because it showed the potential of being a serious flaw in the logic, but no-one else cared because the final output was still correct and the unit tests were passing. Eventually the team leader assigned the bug to me to fix just to shut me up.

So, I may have been a bit grumpy as I looked through his code for the bug, I had my red pen out and my notebook ready, to jot down every problem I found with his code, every potential bug, every confusing variable name, every inefficient loop, every violation of the coding standard.

His code was... immaculate. Well commented, well abstracted, good variable names, correct use of language features. It was probably the cleanest code I had ever seen in that organisation. It was flawless.

HOW INFURIATING!

p.s. When I eventually found the bug, it turned out to be trivial in its impact. There was no serious underlying logic flaw, and no-one cared that it had been fixed.

Oddthinking
Well, this was a nice breath of fresh air among a lot of (entertaining) negativity! I hope you made it a point to learn from the guy.
Software Monkey
LOL. I worked with a guy like that - ex-chemist. Great code.
paulmurray
+1  A: 
  1. Implicit Code !!!! - The one design flaw or mistake that most programmers make is to have implicit "you just have to know code". a programmers assumptions are baked into the code. This is why pairing, code reviews, etc are needed . .someone programming alone can do lots of damage . . .

  2. Classes with multiple responsibilities - The second is classes that have multiple responsibilities. People who code without unit tests often evolve into multiple responsibility classes that are coupled and hard to maintain.

ooo
A: 

People who stick to the framework's way of doing things, even when it's not the best/easiest way of doing things. A lot of times you are trying to do things the framework didn't anticipate, so you end up doing tons of hacks just to get things to work right. When really, if you just forgot about the framework, you could have done everything much easier. This is kind of a counter point against

people who develop .NET like classic asp apps

People who just blindly follow the framework, and don't look at what the best solution is to solve their problem. In almost every instance where I've used ASP.Net, I've never used the webcontrols and viewstate tools that are provided by Microsoft.

Kibbee
Frame works are designed in a certain way, chances are that if you understand and follow the philosophy throughout your projects, it will give you a smooth development process. It comes down to consistency, I guess. When you don't use existing components, you might be reinvent the wheel anyways.
Haoest
A: 

What irritates me most about maintaining other's code is when they have a unified framework for certain specific code, like form output in a CGI application, and then use the standard output buffer to instead make custom output. When restyling or looking for code that breaks, this exception to the status-quo is always the trouble. And whenever the HTML markup or stylesheet changes, every exception must be checked for consistency.

The Wicked Flea
+1  A: 

I could write a book on this topic, but what about inconsistent function naming?, e.g. having all variants of :

functionName

FunctionName

function_name

Function_Name

littered throughout a single file. Throw in the above method for naming files while you're at it.

does it matter ? .. hopefully you are not reciting code over the phone.in fact, i won't recall the code exact casing of function .. just as i would not recall what i ate for lunch...
Vardhan Varma
A: 

Poorly formatted code is definitely up there. I've been having to deal with dynamic SQL producing stored procedures and everything is collapsed and left-justified so it is REAL hard to find all the joins and relationships in a hurry.

Inconsistent code is probably my next ulcer inducer. Ok, I understand if you have too many cooks in the kitchen, you're bound to run into variations and different points of view. Fine, I get it. But when you have something like a CMS and everything is CRUD, and you already have say 10 resources within the same project, 100 resources from another project you could copy/modify, was it really necessary to engineer new code? Sure, I'd love to do things right all the time too, but when you have limited time and funding, "git 'r done" is acceptable.

All time favorite, "New tool syndrome". I forget the exact wording, but whenever a developer reads an article online, say, about Ajax and how you can generate dropdowns, now suddenly, EVERY dropdown on the site is done in Ajax... even a boolean True/False.

Or the person who never actually learned the language they are working in, and ask me how to determine a way to remove add/remove commas to make a string list proper, when the system already provides a ListAdd, ListDelete... /palmface

Adam
We agree that misuse of technology is evil and infuriating, don't we?
Haoest
+3  A: 

Finding really neat code that I didn't think of first!

Ken Paul
To me that's an enlightenment :)
Haoest
+1  A: 

Being borderline evangelical about accessibility, semantic markup, easily readable script and well grouped style information, spaghetti code that mashes content, design and interaction together drives me up a wall. Things like DOM id's and classes that better describe what something looks like rather than what something is. Stuff that locks you into the design. I do realize this is more my issue and neurosis than anyone else's thing, but I really don't want to give up that bit of elitism.

But most recently, completely unindented markup.

mltorrefranca
A: 

People creating several versions of the same file and only using one. All with completely irrelevant names.

We have a php page that tracks emails called default.php. We also have stuff.php and crap.php that do the exact same thing. I spent 2 weeks debugging the wrong one of course thinking it was the right one.

Note: the unprofessionalism of all this is due to all of this being done by interns. Me being one of them. But even I know the value of naming files appropriately.

The.Anti.9
A: 

Aha-code or code you have to be clever to follow. It may look like you're incrementing a counter but in reality you're removing objects from a collection while accumlating a register or something stoopid. I'm to tired to post an exampke but you know what it's like when you hit AHA code.

Then there's the NIH (Not Invented Here) fanatic that wants to re-implement everything from the lowest level because Java regex is broken, or because we can save 4 bytes if we roll our own gzip algorithm.

Also I can't stand static references and tightly coupled logic that encourages copy/paste. When you see a method body a page or so long then you can't use the method anywhere that the stars aren't aligned. You make a change to the callee and it breaks everywhere. You try to break it into a separate project but then you realize you have to port all of it's dependencies as well even if they won't be used in your particular use case.

Cliff
+1  A: 

Reusing the same variable for different, unrelated purposes inside the same massive method as if there were a shortage of the things.

Hank Gay
I used to do this all the time. There was no other way. (Wait, now that I think about it, I still do :)
ldigas
+8  A: 

When the code they give you doesn't even compile

Peter Walke
My coworker hates me sometimes. I develop in .NET, and because of file hierarchy reasons, I reluctant to check in the csproj file, and freshly checked out projects cease to compile...
Haoest
@Haoest, I would hate you too.
Simucal
A: 

Its got to be when the code is just obviously very inefficient, and suffers from the copy and paste syndrone, thus causing you hours of pain to maintain just one thing.

Another good example is vba, where all the code in Excel just uses Offset(x,y) so inserting columns breaks the entire "application".

simonjpascoe
+3  A: 

1) Code that is not written to be self-explanatory (read this http://www.codinghorror.com/blog/archives/001150.html)

2) Massive amount of code in a single file, class or method

3) No coding standard. A bad standard is better than no standard.

Ather
I'm not so sure about (3).
Peter Wone
A: 

I've just been moved to a live WinForms project which has been going on since 2003.

  • The main form where all the action takes place is called Form1.
  • We support multiple document types. Each type of document is handled within Form1 itself using a documentType flag.
  • There is no separation of Model and View. Form1 serves for everything.
  • Cursors are being loaded/set in the OnPaint... No wait! There is no OnPaint override; they've chosen the add an event handler instead of overriding OnPaint.

All of the original programmers have left long ago. Management still thinks that a rewrite is a bad idea.

Vulcan Eager
rewrite may be a bad idea. refactor instead.
Anthony
+7  A: 

Here's a classic: When I see a base class handling stuff for its derivatives based on a 'type' member.

Vulcan Eager
+2  A: 

The thing I hate the most is when the delivered code does not work as promised - when it should implement a certain functionality but only the most basic paths of execution are covered.

This infuriates me because I know that what has been implemented is generally the tip of the iceberg and I know I have to implement the rest of it (and people who are aoutside of the project will think I'm just loittering with code instead of delivering real functionality).

rshimoda
+3  A: 

Putting tabs after the equals sign when assigning values to variables so that everything lines up.

geometrikal
I do that - I find it makes the code easier to read.
Treb
I often put whitespace before the '=' to line up consecutive related assigns; I too find it makes the code more readable.
Software Monkey
Software Monkey: I'd think so too. Depends on the font as well
Jonta
I do this 'symmetrcial coding .. lining up things... IMHO, that make is more readable, and more robust too. it's difficult not to spot accidently placed == instead of = for example.
Vardhan Varma
+2  A: 

Having to correct a bug in a "shared" component.

And by "shared", I mean "shared" as in "Hey, everyone touch this component and add his/her prefered feature to it."

It has an orgy-like smell that makes me gag everytime I have to delve into a less than stellar code which grew by random iteration and patching without a clear vision of what it was supposed to do ("specifications are not mandatory, you know").

You know, the kind of code where there are more ifs and switchs than variables and functions. The kind of code proving that a code quality of a component is of the level of the less skilled developer that once contributed to it. The kind of code that makes your eyes cry tears of blood, and wonder if you should not become Serial Killer instead of Developer...

Because, when you "touch" this kind of code, your greatest wish is to have it work the same way as before but for your little corner case (thus, you add a bunch of ifs atop a mountain of ifs).

Because your worst nightmare is to have a bug assigned to you because you were the last one to modify the code. Or even to look at the source. Or even, to use the component (This is the moment you lose your innocence).

Having to correct a bug in an unmaintained "Core Component"

In "our" team, we have a bunch of in-house developments. Like Joel Spolsky said, "if a module is the core of your business, then do it yourself". Apparently, everything in my team is the core of the business.

To make the taste more spicy, until recently, everything was developed using Win32 API (again, we can't trust MFC or whatever amateurish external library like QT).

So far so good...

But mix it with the "shared" philosophy mentioned above, and the fact that the author of the component did quit years ago and that no one is now (or feels now) responsible of the component.

you know, the naive faith of some non-technical managers about the fact developers are interchangeable monkeys...?

Welcome to the zoo... (Real life example)

This is a real-life example. It happened this week, in fact (truth to be told, it is a real core component, which adds its own perversion to the subject).

In the current case, one year ago, I mentioned the fact we would lose less time by using already existing libraries. The counter argument used was that we did not control the code of external libraries, nor the speed of their bugs correction, and thus, it was better to have our own code, as we could always correct any bug when needed.

Some days after (destiny has its own sense of irony), I discovered one of the core components had such a bug. This bug was serious, but it would only express itself in some corner case situation. They tried to assign its correction to me, but as I was overbooked (Lucky me!), no way I would spend days or weeks in 20,000 lines of old C code using obscure Win32 API functions you don't want to know they exist.

One year after, and guess what, the bug was still there. So much for the "Hey, we develop in-house because we can correct it fast."

And guess what? Someone outside the team saw it.

And guess what? They assigned it to me.

paercebal
"the naive faith of some non-technical managers about the fact developers are interchangeable monkeys...?" Says it all. The big question being why do we allow our team to let the code-base become a monkey-code-base?
Hace
+10  A: 
  • I'm not sure why, but Hungarian really gets my goat. I just can't stand it.

  • I hate it when people don't name things well.

  • And I despise over-architecting. My last job was working on a large, years-old, conglomerate system that exemplified every bad practice I'd ever read about. It was spaghetti code in the pure sense.

    My current job is working on a system that was designed by a self-proclaimed guru who over-architected everything, rewrote major portions of the .NET Framework because he "knew better than Microsoft", and completely ignored decades of advancement in database theory.

    I am constantly longing for that old spaghetti-code system. It was at least grokable.

sliderhouserules
Joels article on this is interesting to read http://www.joelonsoftware.com/articles/Wrong.html
Greg Domjan
+1  A: 

From my current project: Where the (C++) coder gets embarrassed about the huge class he's writing and decides to 'refactor' by cutting and pasting the member functions into 13 different source files. Corollary: When the same coder, confident now, applies the same technique to the other bloated classes in the system, so that one source file may contain fragments of three different classes.

fizzer
+1  A: 

Finding that code doesn't work at all or as expected!

begray
+1  A: 
  1. No Comments
  2. No Encapsulation - everything public
  3. Misuse of OOP - big-ass switch in base class to make decisions that belong to subclasses
JohnIdol
+1  A: 

Something like this for the main working thread of an application that I had to write a new plugin for:

bool WorkNotDone = true;
while (WorkNotDone)
{
    try
    {
        // Load Plugins
        // Plugins do a lot of complicated things here
    }
    catch (Exception ex)
    {
        Log("An exception occured: " + ex.Message);
    }

    WorkNotDone = false;
}

Took me days to find out that my plugin was throwing an exception every once in a while, because the exception got caught and written to some logfile in some obscure directory somewhere in the project...

Treb
+6  A: 

Global variables.

danimajo
Amen! The other day I found a tutorial on the internet on how to create global variables in C#...
Treb
I've seen a lot of global variables named X or x ... Nightmare!
danimajo
+1  A: 

Unnecessary abstraction. I think some people feel the need to apply every OO design pattern to every problem and encapsulate everything, regardless of how unlikely it is to change. Abstraction always comes at the price of making the implementation more difficult to understand, if you need to reason about its performance, fix a bug in it, etc.

dsimcha
The problem isn't the heavy application of design patterns, it's the GRATUITOUS application of them. I wrote some code recently uses six major patterns in 300 lines of code without being particularly abstract. I didn't expressly choose to use patterns, I noticed them after the fact.
Peter Wone
+2  A: 

Similar to some of the above posts, not understanding the technology used is a real problem. I had to work on one outsourced system that was where the code could've been straight C, but was proudly claimed to be C++ by nothing more that changing the file extension!

Trying to get these clowns to use the STL was also quite an eye opener, their idea of using an STL map was to copy the STL map header file, then go and hack their copy of the header file to contain the data types they wanted!

+5  A: 

CoWorkers who still use .net as if it was vb6. IE. just about all code in the form load and behind button_click events and hardly using classes :(.

Morph
See that all the time.
Nathan W
+1  A: 

1.) Stupid variable names, like i or x,y,z

2.) Untested code that breaks the build

3.) Classes derived from derived from derived from derived from ... finally and abstract class

4.) GOTO

5.) Try ... Catch used for logic

hmcclungiii
My rule: the length of variable name should be proportional to it's scope.
Vardhan Varma
+11  A: 

When programming in Java or other languages that have a Boolean data type, I find annoying when people don't treat it that way, for example:

something like this

Boolean istrue;
if(bar<foo)
{
   istrue = true;
}
else
{
   istrue = false;
}

whcih could easily be replaced by:

Boolean istrue = (bar < foo);
Mark Lubin
a blatant sign of noob programmer.
Haoest
+11  A: 

What infuriates you the most when maintaining others’ code?

I can't believe I'm the first person to say this, but having to maintain other peoples' code instead of writing my own is infuriating. Everything else is just salt in the wound.

/And yes, I know that I'm being unprofessional, childish, and immature. But honest.

JPLemme
+8  A: 

I hate when people write functions that do 2 things, like:

public void DeleteUserAndSendEmail()
pablito
I agree 100%, with the caveat that the above function should still exist if the combined functionality is needed often (especially if both operations take parameters). It should be implemented as calling the two separate functions.
rmeador
+1 for the funny function name.
Cam
+13  A: 

Hundreds of lines of commented out methods and other code interspersed throughout the actual code. I once turned a C# class from a 2200 liner to 400 lines in about 5 minutes.

Clearly the offending developer had not realised he had a full version history in SourceControl should he ever need it.

Blatfrig
Wish I could upvote more than once.
Software Monkey
I've actually had arguments with coworkers about that. "But we might need that code in the future!"
Jesse Weigert
+1  A: 

when a function has 20+ parameters.

and better yet, when those parameters start to get numbers on the end of them.

cost1, cost2, netcost1, netcost2, etc.....

Dano
A: 

White spacing. For some reason, code that's done in any other tabbing style then my own drives me up the friggin wall.

watchwood
Just remove the tabs. Under CSH try: alias use-emacs-to-untabify-file '\emacs --batch --eval='"'"'(progn (find-file "'"'"'\!:1'"'"'") (untabify (point-min) (point-max)) (save-buffer))'"'"
Mr.Ree
A: 

Code with no documentation, no one at the company knows who wrote the code or when, it doesn't exist in source control, and some end user is complaining about something changing between yesterday and today without saying that they are using different parts of an application where the whole thing may have been broken for a long time.

JB King
+3  A: 

Typos in variable/db field names.

lush
I think what the OP originally meant was "spelling mistakes" instead of typos. I don't see why this one was voted down. This is a pet peeve of mine too.
rmeador
A: 

In java, useless interfaces. I'm working on something now where someone thought, a few years back, that everything should have an interface. There are probably a good 100 or so pointless interfaces that only have 1 implementation. There was never any real need for it, someone thought it was good style.

 public interface IPointless{
   public String getName():
 }

 public class SomethingNecessary implements IPointless
 {
   public String getName(){ "I am necessary"};

   ...

 }

 public class SomethingVeryVeryFarAwayInADifferentProject {
  private IPointless foo;
   ....
   log.info("It's important that you know that "+foo.getName()... 
 }

Extra points for misuse of spring dependency injection, so all you have is a reference to something whose semantic meaning has disappeared. So anytime you want to trace you have to trace to the config, find out what they're actually using, figure out where the implementation is.

Project actually has a front end to a properties file that has a hard-coded getter for every property, and an interface to the hard-coded functions in the getter. So there's a good thousand lines of code to read properties, spread through property files, an interface and an implementation.

Steve B.
God, yes. In particular: making Hibernate beans implement interfaces. A persistent bean is not a service. It is a bean. The stuff you have to do to add generic declarations to things reveals what a bad idea this is.
paulmurray
+5  A: 

Given that I work mostly in the C++/Java/Perl world:

Global variables: They are the plague! Use them as a last resort!

Goto's: fall into this category too! Mostly for their tendency to create the most dreadful spaghetti-code.

Non-descriptive variable/function names: Really! Don't abbreviate & comment it. I don't want a lookup table. That's what the name is for in the first place! The point of code is to communicate effectively with other humans. Binary is for machines.

Abbreviations in variable/function names: You know what they mean. Nobody else does. (Bonus points if they are in your native tongue, and nobody else on the team speaks it!)

Similarly named identifiers: The joy of trying to discern FooBarCharlie and FooBazCharlie throughout your code really makes my day. (Bonus points when combined with abbreviations and multiplied: ABDF-HGIK-LMNP-STVW, ABDF-HGIK-LMNQ-STVW, ABDF-HGIK-LNNP-STVW, & ABDF-HGIK-LNNQ-STVW.)

Unrolling Loops: Trust me, the compiler is better at this than you are. You are just going to introduce some really painful bugs!

Excessive Complexity: If you have 20 lines that start out "foo(...).bar(...).charlie(...).delta(...).echo(...).", then for gods sake use a reference or a macro to make the code human-readable.

Lack of discernible design: If I can't tell what you are doing, this is not going to go well. Put some UML in the comments! Document!

Multiple names for the same concept: Use xColumnWidth and yRowHeight if you have to. But don't use (x,y) for one method, (column,row) for another, and (width,height) for yet a third! (Bonus points if you reversed the order between methods, mixing (x,y) and (row,column).)

Cut & Paste coding: Really! Others have said this too and I'll say it again! Don't put the same broken chunk of code in 10 different places. Some day, I'm going to have to fix that. Or maybe just change it. Use a macro or a static function!

Shadowing variables: 'Nuff said.

Fixing compiler warnings by commenting out -Wall in the makefile: Me, you, and Mr. Bat need to have a little talk!

Lack of comments: If you are going to put some hairy piece of math into the code, document where it came from! At some point I'm going to have to fix it! (I know, I'm hitting commenting twice. But it's important! Especially when your pulling math out of a book or matlab. (Just give me a prayer of a chance here! That's all I need!)

Checking in broken code: Just because it compiles doesn't mean you are done!

Core leaks: This is going to come back to haunt us... Or, more precisely, me!

Sadly, all of these have been experienced first-hand.

Mr.Ree
A: 

A lot of folks have complained about whitespace issues.

A simple fix, under CSH (UNIX/Linux):

alias use-emacs-to-untabify-file '\emacs --batch --eval='"'"'(progn (find-file "'"'"'\!:1'"'"'") (untabify (point-min) (point-max)) (save-buffer))'"'"

For bonus points, automate it during repository checkin.

Mr.Ree
A: 

In some dank corner, I found the source for "the editor". The editor that everyone said was a great thinkg, although no-one could show me a working copy of it.

The source had a build file. The build file had a target "build webapp".

What it didn't mention was that part of the build involved generating the database tables from the turbine config, and this involved dropping the existing tables.

Oh, and the build (naturally) was configured to run against the PROD instance.

paulmurray
+1  A: 

Along with the other great comments ('clever code', over complicating simple tasks, etc), it 'infuriates' me when the other C# developers (with 7+ years experience over me) don't understand my confusion trying to understand their code structure. For example, single files stored in a deep hierarchical folder structure C:\VisualStudio\WebSites\OurSite\Common\Products\Data\DataSources\Db\SQLServer\SqlServerDataSource.cs

Nearly every file is configured that way (and each project has an identical Data\DataSources\DB\SqlServer folder) with similar 5+ level namespace hierarchy (which doesn't match the folder structure)

Why not simply \AssemblyProject\MyClassFile.cs? Files easy to find, easy to maintain..why so complicated?

When I get to that level of experience (I have over 13 years of Delphi development experience, other developers are from different backgrounds, come from different companies and all have the same mindset)..will I start 'thinking' like that? Will I loose the K.I.S.S. way of doing things? I hope not.

KevinRF
+16  A: 
try {
 // 1000 lines of code handling lots of different tasks
} catch(Exception ex) {
 // nothing
}
Sander Versluys
+1  A: 

I totally feel the misuse of technology. All stemming from a lack of caring and failure to improve. When a brain becomes stagnant, and I can tell by reading your code, it is a sad day.

A: 

I've had to do a lot of this recently, so I'll be honest.

  • Variables declared at the top of the function, uninitialized, in C-style C++ code. This hasn't been required for years and has no benefits, especially if you're not initializing them. It's redundant and pretty much asks for unused variables if the compiler warnings for that aren't turned on. Restricting the definitions to the smallest scope necessary can make odd logic flow in the function more obvious. It's a good change that was made in C99 and C++, and should replace the old method.

  • Uncommented, illogical program flow. Often I see pointless infinite loops, gotos, and longjmps all over the code that make no real sense. They all serve a purpose, but if you can't write complex program logic in a way that makes sense, you should at least comment it.

Dan Olson
+2  A: 

Business Logic and Data Access code mixed in with the GUI code (in asp pages, or even worse aspx.cs files). That really irritates me.

And, to agree with an earlier answer, neat stuff I didn't think of first :)

Antony Scott
A: 

1- functions that passes implementation to others functions for no reason 2- Long 1k > codes that soar your eye 3- Missing dataflow and logic 4- Code written by some1 who never had a programming language and started with html design tool 5- Code the uses includes of chunk of code from different files as opposed to functions calls 6- Highly coupled codes

A: 

Commenting out large bodies of code using something like if(0) {...}. I read the whole thing, figure it out a bit and then see the if block. It didn't help that the 'commented out' block was not indented either.

Noufal Ibrahim
+2  A: 

I very much dislike it when I find that a developer has reinvented the wheel in the sense that a perfectly good library (or the platform itself) offers the functionality required. For example, I have seem developers write their own XML parsers, ArrayList (which had much worse performance), diff system, and database library (for working with databases).

JasCav
+1  A: 

When there are no tests. How am I suppose to be able to change code that I don't know if there are no tests. How will I be able to know if I broke something else.

So please gentlemen, write more tests please. It's better for everybody!

Jonas Söderström
A: 

Classes without valid comments.

Anecdote, from today. I have to refactor a small module that uses a class that is a direct copy of another class from the API of the CMS we program against. Comments only indicate where it's used (not what it does, mind), and I have to fix it because else the module will not be certified.

Fix it, test it... doesn't work, the API alternative isn't available because the module it's in (OSGI) doesn't export it.

Next time you copy a class from the API, please indicate /why/ you copied it, so you don't waste my time.

Second, people that don't read what should be done. Same certification thing, the first round fails, the certifier says "You need to fix this problem, which happens (for example) in this bit of code. You could (for example) fix it like so-and-so".

But what happens, the guy that has to fix it (I just coordinated (past tense)) /only/ fixes that one example line of code, not the actual problem, even though the problem itself /and the whole solution/ (a single line and some edits further on) is posted right there.

Yes, the certification report is sixteen pages of stuff you can skip, but does it really cost you too much time to read that one page for the actual solution and the thing you have to do?

/vent

Cthulhu
+1  A: 

static, there's just so many other much more elegant solutions to it nowadays (this is a Java gripe, btw)

Esko
A: 

Missing indentation.

Thomas Müller