Coding mistakes that are telltale giveaways of an inexperienced programmer
[+165] [120] JohnFx
[2008-10-26 00:12:49]
[ coding-style ]
[ http://stackoverflow.com/questions/237241] [DELETED]

What coding mistakes are a telltale giveaway of an inexperienced programmer?

More specifically, what types of mistakes do you most commonly see in code from really green (inexperienced, not the Al Gore kind) programmers?

(11) Perhaps it would be a good idea for users to add why these mistakes are wrong, just in case some of us (me) look at this page and feel like the community here have been watching me code for years. - Mike B
(12) +1 for the Al Gore differentiation - Tim Büthe
[+276] [2008-10-26 00:14:29] lucas
if (test) {
return true;
}
else {
return false;
}


Good one! I have seen that a lot from the new guys. - JohnFx
Edited my answer to reflect an all too common variant on your comment... - Dave Markle
(1) This is a classic one! - Camilo Díaz
Umm... missing close brace? - user9282
(3) Rewrite as: return test; :) - korona
(2) I mostly see this one in the form: return test == true ? true : false; - Firas Assaad
Wow... a classic, super answer!! - Shivasubramanian A
(44) Ofcourse this should be: bool result = false; if (test) { result = true; } else { result = false; } return result; // now go, get a napkin to wipe your screen... - EricSchaefer
(92) I've seen experienced developers doing this for clarity, or when they expect to have more there. - Uri
This is my worst enemy. I cry when I see this in a mature project... There's no excuse for this. - Peter Perháč
I've seen developers who have been working for 20 years do this. And, as far as I can tell, they aren't the types who are doing it for clarity. Makes me sad/frustrated. - TM.
(49) "When they expect to have more there" means that they're not familiar with the concept of YAGNI. - Andy Lester
(57) Thinking the posted code is clearer than "return test" is another sign of an inexperienced programmer. - Dour High Arch
(15) Readability > Number of lines. It all depends on the exact code. If the 'test' variable is something like 'doesMeetCriteria' or 'isToad' then it may be ok to just return the value. When used properly there's no issue with this. - TJB
I used to do stuff like if (test == true) { doStuff(); } all the time when I was in the intro classes - Alex
(2) I think it depends on the language and usage. for example, sometimes we want to return strictly a boolean. Also, what if test is an object? We shouldn't return the object but instead just return true or false instead. - 動靜能量
This is not inexperience, this is stupidity. However, one may do that if there is more processing in the branches, then it is more clear to return a constant than the original tested value. - J S
@Jian Lin In C or C++ I agree. In C# a boolean, is a boolean, is a boolean. So its just over verbose doing if (test) { return true; } else { return false; }, return test would serve the exact same purpose and be just a clear. - Sekhat
(1) Or when they do something like "if (true == test) // avoid possible assignment to test"... - GalacticCowboy
(14) Sorry guys who voted this up, but this can hardly be called a "mistake". Please re-read the question. In most cases, this shouldn't even generate code that's any less efficient than "return test". If writing it this way clarifies the logic (depending on actual context) then write it this way. IMHO, it's an inexperienced programmer who might think this is a "mistake" or inefficient. - Dan Moulding
(2) This has one advantage: You can breakpoint one of the branches of the if(). I've changed a "return test" into this construct in order to set breakpoints before. Once I accidentally committed it. - user9876
(6) Conditional breakpoints, anyone? - Michael Myers
(3) Of course, an experienced programmer would write: return test ? true : false; // :) - Danko Durbić
(1) While this particular piece of code is probably always a poor idea, some languages require similar constructs if you want to return a boolean regardless of the type of "test". - LKM
(1) @TJB then they shouldn't be using the word "test", they should be renaming their variable to something descriptive rather than expanding to this if/else construct. - TM.
(3) It's obvious what's wrong with this code: Opening braces should be on separate line as per JSF AV rule 60. - squelart
(2) This needs more context. - Tim Post
LKM: If test is not boolean you can make it boolean in all of those languages. - jmucchiello
this one is bad.. but I've got a coworker who insists {type * val = new type(); type->doSomething(); if(val) delete val;} is just a style choice. Our heap is pummelled with these "style choices" all over the fraking place. - baash05
(2) I would do this in case I needed to set a breakpoint on either the true or false situation. After debugging/testing it I may forget changing it to the clean form of a simple return statement. I assume it doesn't matter because the compiler should be smart enough to create the same executable code. - Niels Basjes
(1) How about in SSRS: =Iif(Fields!BooleanField.Value = True, True, False)? This makes me cringe since of course it should just be Fields!BooleanField.Value. Excessive garbage compiled away or not, it demonstrates a serious thinking problem on the part of the person writing that obscenity of an expression. - ErikE
1
[+208] [2008-10-26 00:22:26] Eoin Campbell

bool x = SomeMethod();

//Sometime later

string whatIsX = x.ToString().ToUpper();

if(whatIsX == "TRUE")
{
//Do Stuff
}


Niiiice.. me like ! - Newtopian
(14) Eew, just looking at that makes my toenails curl. - HS.
Off to the Island of Misfit Mascots Commune for you! - TraumaPony
(120) That's not 'inexperienced'... It's just horrible. - Ace
(3) ooooohhh... I have seen many ugly lines of code involving boolean logic, but this is by far the most... ineffective. - Peter Perháč
(15) Yeah, that's awful. I'm working with a Senior Developer who writes stuff like this. It's frustrating that he gets paid more than I do. :( - Greg D
This seriously makes me cringe. I can hardly believe someone actually wrote this and wasn't joking! - Noldorin
If you liked this, you'll like thedailywtf.com/Articles/The-Test-of-Truth.aspx - Greg Beech
I'm with Ace. I have never even thought to do something like that, even when I was about as good at programming as my cat. - Andrew Noyes
(3) is this real? or just made up joke? - ra170
(1) That wouldn't make me sad, that would have me laughing all day long. - Beska
It would be even worse to see this: select case when upper(cast(boolcolumn as varchar(10))) = 'TRUE' then 'TRUE' else 'NO' end as [SoBadImCrying] from table - cjk
. . . . . . . . - Binary Worrier
(1) That was me being speechless . . . - Binary Worrier
(17) What? He doesn't use UpperInvariant!!! - kentaromiura
lol, I just made a similar version of this and sent to my workmates asking for "advice" on my design ^_^ I'm looking forward to their responses hehe - Kenny Cason
2
[+140] [2008-10-27 03:48:20] johnc

Letting the compiler do your testing - "it compiled, it's OK"

(2) Indeed, this is a real sign of inexperience. - mafutrct
(10) this is classic! this should have more votes. maybe people who use dynamic non-compiled languages just can't relate to this one, resulting in fewer up-votes. - que que
(2) @que que: This is far worse in dynamic, non-compiled languages. The runtime hasn't even necessarily gone over all code-paths, like a static compiler would have had to. - Bernard
(2) What's wrong with that? I set my compiler to treat warnings as errors, after all...</sarcasm> - Nathan Clark
(5) In Haskell or CAML, it's true! - Dario
(2) Dario, really?. - Stefan Monov
(1) Dario, they can simulate all possible inputs and check the output for correctness? I guess all the testers will have to look for new jobs then. - Georg Fritzsche
(1) Hey, that's the first test, isn't it? - Kirk Broadhurst
3
[+138] [2008-10-26 02:17:53] user25306

OK, so I was bored ... and in truth, some of these only mean you are an OLD developer :)

If your code looks like it was written by a committee, you might be an inexperienced developer.

If you put comments on every other line of code, you might be an inexperienced developer.

If you have never spent more than 4 hours debugging something stupid and obvious, you might be an inexperienced developer.

If your code has nested goto statements, you might be an inexperienced developer.

If you still write in a language with “basic” in the name, you might be an inexperienced developer.

If you have never seen the sun rise and set and rise again while working on a project, you might be an inexperienced developer.

If you don’t have a religious opinion on software development, you might be an inexperienced developer.

If you use Thread.Sleep() to fix race conditions, you might be an inexperienced developer.

If you learn something new, then immediately apply it to EVERY PIECE OF FRACKING CODE YOU WRITE, you might be an inexperienced developer.

If you think you are too good to write unit tests for your code, you might be an inexperienced developer.

If you have not (yet) learned to despise Hungarian notation, you might be an inexperienced developer.

If you have learned to despise Hungarian, and still can’t intelligently argue why it should be used, you might be an inexperienced developer.

If you have to fix warnings to compile your code because the compiler treats more than 1000 warnings as an error, you might be an inexperienced developer.

If you think design patterns are the Holy Grail for software development, you might be an inexperienced developer. (Or a manager)

If you don’t have at least 15 books on programming that you have never read, you might be an inexperienced developer.

If you think you have never been guilty of all of the above, you ARE an inexperienced developer.

If you don’t know who David Ahl or the Beagle Bros are, you might be an inexperienced developer.

If you have never developed software on a team where everyone was smarter than you, you might be an inexperienced developer.

If your eight year old kid debugs your code, you might be an inexperienced developer.

If you can’t name at least 50 things wrong with the win32 API, you might be an inexperienced developer.

If you have never argued with a tester about a bug that is “by design”, you might be an inexperienced developer.

If you have never developed on a mainframe, you might be an inexperienced developer.

If you have never written anything that uses ASCII graphics, you might be an inexperienced developer.

If you have never tried to convince someone that C# is better than Java (or vice-versa), you might be an experienced developer.

If you can’t divide hex numbers in your head, you might be an inexperienced developer.

If you have never written an application that compiles out to a .com extension (or even know why you would want to), you might be an inexperienced developer.

If you have never written a fully functioning application that runs in less than 1k of memory, you might be an inexperienced developer.

If you don’t know the difference between 8080 assembler and 6502 assembler, you might be an inexperienced developer.

If you have never written software to create music on hardware that doesn’t have any type of sound processor, you might be an inexperienced developer.

If you have never been GRUE or WUMPUS hunting, you might be an inexperienced developer.

If you have never tried to improve "Eliza", you might be an inexperienced developer.

If you can’t relate to any of this, you might not be a developer.

/// ==== EDIT

I noticed a comment on the original question, why are some of these things wrong? Here are some (random) resources. They range from technically useful, to just history...

http://en.wikipedia.org/wiki/David_H._Ahl

http://www.boingboing.net/2006/01/17/dot-matrix-printer-m.html

http://www.humanclock.com/webserver.php (25k, but hey - it's a full webserver ...)

http://en.wikipedia.org/wiki/Beagle_Bros_Software

http://en.wikipedia.org/wiki/Hunt_the_Wumpus

http://en.wikipedia.org/wiki/Eliza

http://www.joelonsoftware.com/articles/Wrong.html

http://blogs.msdn.com/oldnewthing/archive/2005/01/14/352949.aspx

http://blogs.msdn.com/jfoscoding/archive/2005/08/06/448560.aspx

http://msdn.microsoft.com/en-us/library/bb429476(VS.80).aspx

http://code.msdn.microsoft.com/sourceanalysis

http://www.nunit.org/index.php

[3] http://en.wikipedia.org/wiki/Grue_%28monster%29

(10) How do you nest gotos? - paperhorse
(2) I wrote an OS in under 1K of RAM. :-) - Paul Nathan
(1) @Vlion Sticking it in the ROM is cheating! ;) - Jacob Krall
(2) I see you must be going for the longest post... - Chris Lively
(3) @paperhorse: If (x) goto y else { if (q) goto z else if (m) goto(l) else if ( p) goto(a) else goto(b) } - Nazadus
(13) +1 for Thread.Sleep() shudders - RobS
(16) +1 for inexperienced programmers (and managers) thinking design patterns are the holy grail - AaronS
+1 This was brilliant! - Jas Panesar
(1) Beagle Bros--they're those criminals from the Gladstone comics, right? ;) - outis
(15) Oh man, I can't even divide decimal in my head. ;-) - Ferruccio
i vomit a little when i need to work with vbscript, must be the 2x strength coffee... - BigBlondeViking
(8) Many of these questions are about whether you were programming in the early 80s. But that's a long time ago. You could have entered the industry in 2000 and still be a very experienced developer (but please don't call yourself a "guru".) - finnw
The only one I didn't really understand was the a.com part... Guess I'm an inexperienced developer :\ - micmoo
(3) If you call yourself a "guru" at anything, you might be an inexperienced developer :) - Not Sure
(1) +1 Lol at #3. Funny cos its true. - Justicle
(3) +1 If you have never seen the sun rise and set and rise again while working on a project, you might be an inexperienced developer. :)) - Shadi Almosri
(2) #3, most bugs are stupid and obvious ... in hindsight. - Casey
(4) +1 for accumulating programming books that you never read. :-0 - Tuzo
(2) experience != age - Surya
I cant divide hex number in my head:) - chester89
(1) If you don’t have at least 15 books on programming that you have never read, you might be an inexperienced developer. -- Thank you, I don't feel so bad for wasting money anymore :) - Tim Post
(1) -1 for being old. Experienced != OldAndCrusty. Or for the youngsters out there, Experienced is not OldAndCrusty. - Evan Plaice
The only programming book that I have which I haven't read is PASCAL second edition. Though I do have a copy of The Second Book of Machine Language, which I haven't read completely. - Brad Gilbert
4
[+130] [2008-10-26 00:15:09] Dave Markle
• Writing too much code
• Reinventing the wheel
• Not closing/disposing/releasing resources
• Not properly handling exceptions (smothering them/ignoring them)
• Gratuitous overuse of ToString(), sometimes even on String objects!
• Comparing boolean results directly to "true" and "false"
if (IsAManMan == true) {
Console.WriteLine("It's a MAN, man!");
}


I've seen experienced developers do the same as well. Heck, some of it, I've done myself ONLY for my trusted IDE to warn me of what I've done so I could fix it right away! - anjanb
(34) Agree with everything but your last point. I prefer having explicit boolean comparisons whenever possible, it doesn't harm and is good practice, not having them just seems lazy. - Robert Gould
(69) If comparing Boolean results directly to "true" and "false" is a good practice, then why stop there? The equality operator returns another Boolean, so why not compare that? And its result? And so on? if((IsAManMan == true) == true) {...}, if(((IsAManMan == true) == true) == true) {...}, etc. - bk1e
@[bk1e]: lol - you missed the point, that's a bad practice because it's unnecessary... - Steven A. Lowe
(41) It's not necessarily bad practice; it's easier to pick out "if(foo == false)" than "if(!foo)". The exclamation mark can be missed easily. ... Having said that, I use "if(!foo)" myself, but I disagree that the above is bad practice. - Rob Howard
(1) It's not ALWAYS a bad practice... But it often betrays a lack of understanding of what boolean variables are and how to use them, if it's peppered everywhere throughout the code. 99% of the time a god variable naming convention (like "Is" or "Are" for booleans keeps everything nice and clear. - Dave Markle
(3) Comparing a boolean variable to true / false actually creates 2 boolean operations. You are comparing (var == true) which returns a bool, then you compare the result implicitly in the if statement! Totally unneccessary. - Mark Ingram
(3) @Mark Ingram: Most compilers are smarter than that, the test is done once and a jump is taken or not depending on the result. I'd agree from an aesthetic perspective, though. - Firas Assaad
(6) Just for the record, ((object)someString).ToString() has the nice feature of converting nulls to an empty string, so it makes sense sometimes to use ToString() on (potentially) string objects. - DrJokepu
(13) @DrJokepu - No it doesn't, it causes a NullReferenceException. If you want to convert a potentially null string to an empty string then use the null coalescing operator, i.e. somestring ?? string.Empty. - Greg Beech
Comparing to false is useful in finding out if an object is not of another type, e.g. if(myObj is MyType == false) works the same as the fabled "is not" - cjk
(1) I think reinventing the wheel is by far the biggest tell. Especially when the wheel is something that's commonly found in the standard library of almost every popular programming language. It's a clear indication of a lack of knowledge of the available libraries. - Kamil Kisiel
Particularly problematic is when booleans are infact integers (as it is in C), yet programmers still compare against true and false explicitly. The example here is in C#, so that is not the situation here, however, in languages such as C, it is conceivable that one could have an erroneous value '3', which is not true (perhaps defined as 1), nor is it false (defined as 0). - Arafangion
Actualy, sometimes adding "== true" in expression can increase code readability. - Marko Kocić
OTOH, you can't unintentionally write a assignment when you intended to compare two boolean values by leaving out the explicit comparison: if (redButtonPressed = true) { launchTheNukes(); } I didn't do it ;) - AdrianoKF
str.ToString().ToString().ToString().ToString().ToString() - bobobobo
(1) Keep in mind that the Standard C functions available in ctype.h (isalpha, islower, etc) commonly return "true" values that are not 1. Writing if(isalpha('x') == TRUE) will bite you. - Dewayne Christensen
5
[+117] [2008-10-26 00:18:32] Decker

Using lots of global variables

(2) Global variables have been banished and replaced with singleton classes. - Barry Brown
(15) I might go as far as saying overuse of Singletons is worse than overuse of globals. Or it could just be the guy who overdid singletons was a worse coder thant he guy who overused globals - Neil N
(5) You should say: "using global states" instead. There are people who seem to have missed that singletons were degraded from PATTERN to ANTI_PATTERN 5 years ago ;) - ivan_ivanovich_ivanoff
(25) Neil, a Singleton is just a fancy name for a global object. - user9876
(1) I once worked for a company where the previous developer had a 4,000+ line file, in Java mind you, called ConstantsAndGlobals.java. Nothing by 4k+ lines of public static final ... - amischiefr
@user9876: Thinking a Singleton is just a global object is a great indicator of inexperience. - Mike Burton
(1) Mike, please elaborate. - Stefan Monov
@Mike: Agree w/ Stefan. You just have to call a function, e.g. "ClassName.getFooInstance()" instead of referring to it directly by name, "foo". - Claudiu
(2) @Claudiu/Stefan With a singleton, you cannot create a new object and replace the one that is in use, which was about 80% of the problem with global variables. With a well-constructed singleton you have a nice clean way to access a single-instance object across your entire codebase. You are protected from initialization concerns and you are protected from replacement concerns. Those two are massive steps forward. The fact that you can access and manipulate the object globally is a feature, not a bug. There are plenty of cases where it's useful to be able to do so. - Mike Burton
I thought the definition of a singleton is that there is only ever one instance of the object. - StriplingWarrior
@StriplingWarrior: that's true. However in today's complex applications a number of people would rather create some global variable (ala a singleton property) instead of dealing with getting proper responses from various method calls. Bad design, difficult to keep track of, and generally wasteful. - Chris Lively
@Chris: Understood. I misunderstood Mike's comments, and thought he was saying singletons were better than global variables. Since I discovered dependency injection, I've come to realize that there are very few places where Global variables are the best solution, and arguably no places where singletons are the best solution. - StriplingWarrior
It's all well and good when you're starting from scratch to discuss using better design techniques. That's not what most of the coding in the world is about, however - you get a few years of maintenance mode under your belt, you learn to be thankful for all the tools in the box. Singletons greatly reduce the complexity of dealing with certain kinds of globally accessible data. - Mike Burton
6
[+96] [2008-10-26 00:29:02] itsmatt

Copying rather than reusing code.

Creating home-brew 'solutions' when framework solutions are available.

Code that doesn't bound-check, guard against generating exceptions, doesn't use exception handling, and is brittle.

(In my shop where testing is key) Not writing test code.

There are other tell-tale signs that aren't in code, of course.

(6) Ugh. Point #1 should strike fear into the hearts of programmers everywhere. - Jedidja
I had a lead once that would c/p swaths of Java code, then when defects popped up, guess who had to fix all 10 copies.. :( - lucas
This is actually a nasty way of playing politics. Your lead has found a way to have your productivity attributed to him whether measured by LoC or function points. - Peter Wone
Agree. Point 1 is the most common I personally have seen. The same code, with just a constant or two changed, repeated n times! - sundar
(15) Copying vs reusing is a tricky one. There are times when trying to generalize code enough to be reusable is far worse than just making a copy and tweaking it. - Jonathan Allen
(2) I wish I could upvote you 20 times for the first two points, particularly the second one. I have a friend who is constantly creating home-brew "solutions" and ignoring framework solutions. - Gregory Higley
(3) I agree on your "solutions" point for production code. But when you want to learn more about how (or why) something works... the best way is to do it yourself. - Matthew Whited
(3) I disagree on the second one. If you are on a short-term project in an unfamiliar framework it can make better economic sense to create home-brew solutions as this will often be much faster than learning that part of the framework. - finnw
(1) Creating home-brew 'solutions' when framework solutions are available. -- Every framework started out as a home brew solution because what existed did not work, for whatever reason. - Tim Post
I have been guilty for point #2 :-/ - Kenny Cason
Frameworks. Ugh. I have yet to see one that wasn't outdated (based on an older version of the .Net framework, etc.) or overly constricted (you're always writing enhancements to get it to do what you need). Sooner or later, you ARE going to be stepping through the code to debug something, so why not just write that code yourself? Then you can trust what it is doing. - DOK
7
[+87] [2008-10-26 04:30:17] Chris Lively

The best of sign of inexperienced programmer is the one who constantly rushes headlong into the latest technology. This person wants to immediately apply any new trinket into the mission critical app they are working on. Incidentally, this is a leading cause of project cost overruns and failure.

The number two sign of an inexperienced programmer is the one who can't abandon their pet solution when it doesn't work. They will spend hours and days trying to coax it to work instead of wiping the slate and changing direction.

That's exactly the answer i was looking for! Sadly it's way down here but now it has one more upvote. Overexcitement about new technology and clinging to their own costum solutions even if they are riddled with problems are tell-tale signs of inexperienced developers. - steffenj
(3) Much more insightful than the examples of coding errors posted in this thread. - Barry Brown
(1) I tend to like playing with new tinkets early on in development, but after the first 1/3, I tend to get very nervous with going to new stuff. The learning curves just lead to too many mistakes. - Nazadus
(2) guilty! I do that pretty often, but not so much anymore, i'll wipe a project, I insist on using the new hawtness tho so i can learn on the company's budget lol - Firoso
(1) Indeed - but "reinventing the wheel" is the other extreme. Where EVERYTHING is re-done regardless of whether it worked or not. Often related to the "Not Invented Here" syndrome. - MadKeithV
@MadKeithV: Agreed. Although I think that anytime you pull code or libraries from external sources they should be evaluated separately and not just immediately thrown into the mix. - Chris Lively
(7) Re: "constantly rushes headlong into the latest technology". I know this is blasphemy, but I try to do it as much as I can. Projects I often work on can often take more than 6 months to release. Just in time for first stable version of the product I am using to be released. The only way to keep up with the market. And I don't remember being burned ever. Even if it happens sometime in future, it will be first time after many such projects. - Dan
(1) "Incidentally, this is a leading cause of project cost overruns and failure." I would be interested in seeing actual evidence of this. - Robert Rossney
If I really want to learn a technology/design pattern, I'll use it full-on in a hobby project. Most of the time, I'll realize it's more trouble than it's worth (ex: MVVM). Sometimes I'll find specific issues with it (ex: IronPython/DLR... performance nightmare for my use cases). Occasionally I'll fall in love with it and beg my boss to be able to use it at work (ex: aspect-oriented programming) - Robert Fraser
The complete opposite (refuse to adopt anything new) is a sign of an inexperienced programmer as well! - Dave O.
@Dave: I think that was covered by JPLemme several posts before this one. I think the gist is that an experienced programmer will try things out either on their own or in non-critical applications. But when it comes time to building something rock solid, you go with what you know will work as opposed to what you think might work. - Chris Lively
8
[+86] [2008-10-26 02:22:05] Orion Edwards

Oh, and this monstrosity: We had a junior programmer who used to do it quite regularly. If I ever am forced to work for a shop insane enough to incentivize based on lines-of-code produced, it'll be at the top of my toolbag:

for( int i = 0; i < 3; i++ ) {
switch(i) {
case 0:
doZeroStuff();
break;
case 1:
doOneStuff();
break;
case 2:
doTwoStuff();
break;
}
}


(17) I had one like this--it was a 16 port system and each port had it's own case. Funny thing is, each case was copy & paste except for the PORT NUMBER! - Bill K
(6) I can't begin to imagine what the rationale for writing this code is... - Max Howell
Good call - I've seen a few of these. +1 - fizzer
(1) WTF? That's crazy. - dpurrington
The for-switch pattern: a dailywtf clbuttic! - c0m4
I've seen this recently - even worse, this person had essentially unrolled the loop by hand (probably thought he was being paid by the LOC...) - Lance Richardson
(7) Nice, the classic For-Switch paradigm! thedailywtf.com/Articles/Switched_on_Loops.aspx - Greg D
I've seen it not only on dailywtf.com but also in production code. I just cannot understand what drugs you have to take to write such code. - ya23
(1) An old system I worked on was originally written by a guy paid per LOC. It included a timer that counted seconds up to 10 minutes, and he used an if/else if block for all 600seconds. That's an easy way to make a buck. - ScottS
(1) Staring at that, I'm just confused. Why would you do that? - ebencooke
I bet the functions he's calling in each case are also for-switch patterns! - T Pops
Eeeewww! That's nasty - Harry Lime
(10) case 3: Break(); break; - Alterlife
(11) That's pretty cool. So he can do a subset of all that crap just by changing the for loop. :-) - Nosredna
@Nosredna, maybe he is trying to emulate COBOL's "PERFORM X THRU Y" statement - finnw
(1) You haven't lived until you're cleaning up the code from the person you replaced who has a switch statement for every letter of the alphabet. I think they were doing something to check to see which drive letters were in use. I say 'i think' because it was too hard to read the code through my tears. - Dinah
I saw that in some code my students wrote and submitted as homework. I wanted to scream, and had no way to tell them loudly enough how horrible it was. I mean I put some notes on the paper, but who's to say they are going to take that to heart? - Karl
At least he did not indent cases under a switch. What you posted makes me want to guide him .. doubly indenting cases under a switch would make me want to shoot him. - Tim Post
Nosredna: That is the only (and marginally) use for this construct. It makes a suite of tests easy to use while you are debugging a specific bug. Normally, it runs all test. But say there's a bug in step 5 but you need step 3 and 4 to run in order to step 5. So you set the loop to 3 through 5. If steps 1 or 2 are long, this saves you time each time through the debugger. Of course, that makes this construct useful in test code. Not in the actual system code. In normal code it has no purpose. - jmucchiello
yeah, the real wtf is that he's using a switch statement, he should create a list of command objects that have the proper encapsulation that way he could foreach over the objects and call the doStuff() method and the polymorphism would take care of everything. - Nathan Koop
I honestly don't see why this is so horrific. It's clumsy and inelegant but nothing to ::gasp:: @. For some reason it reminds me of the classic 'for i in range(10)' example. - Evan Plaice
@Evan Plaice: for i in range(10) is an alternative way of writing for(i=0; i<10; i++). You could argue that one is better than the other, but they're about the same "length" and complexity, so it's really a wash. The 'for-switch' pattern however, is actively useless. It's equivalent to putting arbitrary meaningless int unusedVariable = 1; unusedVariable++; randomly throughout your code - Orion Edwards
From the point where you have 3 things to do, and figure that you should do them in a loop, this is probably the nicest solution you can come up with ;-) - aioobe
(2) "Hmm, I have these three steps. Each of the steps must be executed in order. How do I do that? I know, I'll make a loop where the loop counter represents the step. That way I know that step 1 occurs before step 2. Wait, arrays are zero-indexed? Hmm... Ok, we'll call it step 0...." - GalacticCowboy
I've seen this. =( - Mike Graham
9
[+84] [2009-05-20 18:20:49] community_owned

Not being happier to delete code than to write it.

(28) Nothing better than replacing a 30 line monstrosity with one or two liner. - Bernard
(10) Less code is typically easier to read. I enjoy showing devs that their 4 hours of work can be replaced by an existing framework method. - Matthew Whited
(3) Best answer so far. - Tim Post
(2) I want to + this 100 times - Chad
10
[+68] [2008-10-26 02:17:39] Tim

Most of these seem like bad programmers rather than young ones.

In my experience young ones typically don't have the savvy of using some best practices and coding for maintainability - they usually want to show off their skills and brains rather than making code easy to read and maintain.

Writing bad code is not exclusive to young programmers.

(9) "Writing bad code is not exclusive to young programmers." -- So true. I would like to believe that the young do it out of ignorance. Btw, this comment has nothing to do with the fact that I am only 22 :) - Vijay Dev
(18) LOL, And sadly, IMO people can be inexperienced even if they've been in the industry for 10+ years. Often, 10+ years of BAD experience is worse than none! - Dave Markle
+1. I've seen most of these from people who have been coding for years. - Gabe Moothart
All of us produce horrible code, at least a few times a day. That's why keyboards have a backspace. Its what you COMMIT that matters. - Tim Post
11
[+66] [2008-10-26 08:44:43] Charles Bailey

They've know that C strings need to be terminated with a null character, but haven't yet understood this.

/* Ensure mystr is zero-terminated */
mystr[ strlen(mystr) ] = '\0';


(2) but but... uhh... but... Oh... Uhh..What!? Why would you do that!? I'm so confused. Damn Junior Developers. - Kieveli
(7) That is a really great piece of code for interview! - ya23
(24) uh, doesn't strlen search for a \0? - apphacker
(7) apphacker: I think that is actually the whole point to this tongue-in-cheek answer. :) - Arafangion
(18) whoooooooooshhhhh! - Logicalmind
@apphacker: bingo :) - Nippysaurus
argh! that's horrible! :P - Stefano Borini
Oh good god, you actually saw this? - Tim Post
(1) I've actually seen worse. Someone that tried to null terminate but doing: ptr[x] = "/0"; ... and the Sun C++ compiler actually accepted that. - Mattias Nilsson
HA HA! great laugh for a friday afternoon - knittl
@Charles it's very depressing but I've seen code similar to this (insurance assignment which makes no sense like if(!x) x = 0;) written by senior C/ASM engineers who have been coding far longer than I have, probably as hasty attempted workarounds to strange bugs through a sloppy shotgun debugging process under tight deadlines. - stinky472
You do need to do that for strncpy though. Stupid design of that function. - Ethan Steinberg
12
[+64] [2008-10-26 00:28:27] Glomek

Learning one hammer and then using it for all problems that it can handle, even if it is ill suited. For example, learning how to use a database and then using it for all data storage, even when a file would be more appropriate. Or, learning how to access files and then using them for all data storage, even when a database would be more appropriate. Or, learning about arrays and then using them for all data structures, even when hash tables would be more appropriate. Or, learning about hash tables and then using them for all data structures, even when arrays would be more appropriate.

(9) Real Programmers know that all data structures are syntactic sugar for byte[] and that [StructLayout(LayoutKind.Explicit)] and [FieldOffset(n)] can help you port your precious libraries from Fortran. Real Programmers can write Fortran in any language! - Peter Wone
(7) No, real programmers need just a magnetized needle and a steady hand to work the disk platters but do you really want to go down this road? - Nouveau
I think the biggest criminal offense of using the wrong tool for the job is when newbie programmers go OOP happy and would write their e-mails in OOP if the english language would freaking finally adopt an OOP model. - Andrew Noyes
(1) @Nouveau: Nice, 'course there's an emacs command for that - BenAlabaster
Comment.At("Andrew Noyes").Post(this.Agrees.ToString()); - Neil N
@Neil What a nice API, I want it. :P - Sekhat
(13) @Nouveau: Real programmers use butterflies. (xkcd.com/378) - user9876
(13) Listen XML is like violence. If you can't solve your problem with it - you're not using enough - PSU_Kardi
(2) Bit like learning to use Linq (.NET) and then using it for everything which it seems a lot of people do. - Ian
(1) I've often referred to this as "shiny hammer" syndrome. "When you have a shiny new hammer... EVERYTHING begins to look like a nail." - mmc
(6) Most common manifestation: A programmer that just learned inheritance and realizes that everything in the universe is a special case of an Atom. - JohnFx
Actually, everyone knows that the command pattern does everything. - rlbond
OMG! JSON and LOLCATZ! ... Yeah, very annoying. - Tim Post
Doesn't this post make anybody else feel like parsing XHTML with RegEx? I know I do ;) - Evan Plaice
@Peter Wone +1 I LOL'd @ "Real Programmers can write Fortran in any language!" I'm saving that one. - Evan Plaice
13
[+58] [2008-10-26 08:08:22] pablito

I've actually seen this:

            bool b;

switch(b)
{
case true:
DoSomething();
break;
case false:
UndoSomething();
break;
default:
MessageBox.Show("Error");
break;
}


Ahahahahahahha... oh g_d you're serious - Jacob Krall
A bool switch, and whats worse, no breaks :( - Furis
(2) HAhaha, looks innocent till you realise you dealing with a boolean :) - leppie
(1) Shouldn't that be: maybe: MessageBox.Show("Error"); break; - EricSchaefer
That is funny. - dpurrington
i like languages like Scheme which give you a syntax error when you have unreachable default clauses. - Claudiu
That code wouldn't compile in C#. It's like they got into some of these guys heads or something. - Thedric Walker
Hilarious, I AM relatively inexperienced. - Anthony Potts
(64) The third option should throw a FileNotFound exception... - Chris Lively
(1) +1 for the WTF reference, Chris. - JohnFx
I guess he/she learned Cobol first and tries to do an EVALUATE. - Luc M
(17) What's worse, the default is actually reachable! b takes up one byte and was never initialized. (2 != true). - Joshua
(35) This pattern is important. It allows you to handle the situation where a quantum singularity transforms the laws of physics. - ebencooke
(4) Joshua: I didn't believe that at first but you're right! At least in D, it is possible to create a bool that's neither equal true nor equal false. You'll need pointers or unions to do it though. - FeepingCreature
Are you sure this was the C/C++ built-in 'bool' and not the Windows 'BOOL'? If it's the latter then the switch might really be necessary. - finnw
14
[+47] [2008-10-26 04:18:45] Simon Howard

Probably the most tell-tale sign is an inability to properly factor out code into separate easy-to-understand chunks. If you're regularly encountering functions that are hundreds of lines long, or nested to four or more levels, it's probably a good sign that they're inexperienced.

(8) "functions that are hundreds of lines long" Yeaaaaaaaah, I wish I could charge that to inexperience only. long sigh - Paul Nathan
Once again the double edged sword of "They're also inexperienced if they do it too much" applies. End result: Moderation is the key. - Sneakyness
I have hashing functions that double that, easy. I'm not proud of them .. but splitting them up when only half a dozen other functions (out of thousands) actually call them seems silly. - Tim Post
15
[+36] [2008-10-26 03:50:15] Sherm Pendley

Two giveaways:

Language religion. There is no "one true language," but it can take time and experience to realize that.

The belief that complexity is a virtue.

So all of Reddit are inexperienced programmers :) - FlySwat
How so? For a particular person, there might very well be one true language. One's brain could work that way that he could achieve the best using e.g. very dynamic language. - egaga
For me, my life is pretty much C. If you give me something to write in a day or two, I'm (likely) going to do it in a language that I know rather well. That has not stopped me from exploring Python and OCAML, but I'd hardly consider myself capable of producing production code in either language. - Tim Post
Throwdown: y'all need to convert to Catholicism now. - FastAl
16
[+34] [2008-10-26 01:52:50] JPLemme

Fear of straying from what they know. I had a newbie programmer who insisted on using arrays for everything because he knew how to use them. He couldn't comprehend that Collections were just an easy-to-use array -- Collections were big, scary objects and therefore too "complicated" to use.

Needless to say he didn't really understand how to use arrays, either...

(5) I admit that I never took the time to learn StringBuilder because I thought it was overkill, typical MS bloated, library that wouldnt do me any good. Then one day I spent 3 minutes learning it and wondered why I didnt use it from the beginning of my .Net coding. - Neil N
17
[+34] [2009-05-21 02:51:36] community_owned

Not asking questions when they don't know.

(7) They do ask questions, just not to you :) Typically, its Google. - Tim Post
(1) @Tim Post They do ask questions, just not to you :) Typically, its Google Or the compiler, in the form of please please please let this compile:) - chollida
(2) I think this is more a quality of a bad programmer because he/she will never become very good without asking a lot of questions (even to google). Whereas, I think a master programmer is one who never shies away from giving explanations. The best developers I've ever met were the ones who always explained more than you wanted to know and were impossible not to learn something from. - Evan Plaice
18
[+30] [2008-10-26 04:12:56] baash05

I think the complexity is the easiest way to sniff out a new coder. Experienced coders are in their soul lazy. They want things to be readable and they don't like to have to remember what a variable or type is. They realize that the simpler the code is the easier it is to debug and the less likely it is to break. New coders over complicate things. Another thing that I think a new coder does is re-invent the wheel. Not really their fault they just don't know enough about wheels that were already invented.

19
[+25] [2008-10-26 10:55:23] Ather

When the programmer assumes that everything will work out fine ...

double MyFunction( int intParam )
{
return localVar = 100 / intParam;
}


(75) The real WTF is that your function shouldn't return anything. - Kevin M
+1 for lol - pure genius - John Nicholas
I've seen inline functions and macros that are similar, but at least they change depending on how they were compiled. - Tim Post
If this was PHP, it would've set localVar to 100 / intParam and returned it :) - Znarkus
Wouldn't this return 1 on most platforms? Seeing as how true often translates to 1... or maybe return 0 ... or maybe -1 ... or ... ~ Yeah, just wow. +1 - jcolebrand
(1) Technically I don't even think this would compile. :) - baash05
You can divide by zero with doubles, so as long as you do 100.0 / intParam, you should be fine. - Ethan Steinberg
20
[+21] [2008-10-26 08:43:41] Barry Brown

The single biggest giveaway I've seen? Not planning before coding.

Unfortunately, this applies to intermediate and many advanced programmers, too.

(3) This is why I believe peer review every now and then is very important. Even if it's short, it catches things like that and when one has to re-write often enough, they learn to think first and code second. - Nazadus
As a junior developer this definitely still is a point of failure for me, I sometimes still do this. I think lots of problems just go away with careful planning, the 20% of uml that saves you 80% time - Alberto Zaccagni
One could argue that self inflicted scope creep leads to the destruction of a lot of bad code, as they find that even small changes become rather time consuming to implement. Even with a firm plan, ideas happen while implementing it, which serves as a cornerstone of the QA process. - Tim Post
spending 20% of my time in UML would cause me to gouge my eyes out with my mouse and run screaming for the hills. red/green/refactor is a much better way to enforce planning before you code. - Matt Briggs
21
[+20] [2008-10-26 00:26:20] vt.

class HashTable {
Object[] keys, values;
...
Object get(Object key) {
for (int i = 0; i < keys.length; i++) {
if (key.equals(keys[i]) == true) {
found = values[i];
} else if (key.equals(keys[i]) == false) {
// do nothing
}
}
return found;
}

• Learning how to use regexps and using them for everything
• Using strings and regexps for rounding/truncating numbers

(13) "Reinventing the wheel badly" is also known as "reinventing the square wheel". - CesarB
Awww, I've been trying to user RegEx a lot lately.... - isc_fausto
Woah. Using a regexp to round a number is the worst idea I've heard this month. - Maximilian
Apparently you don't work with floats much in a strictly typed language. Regex to round a number? I'm glad my bank doesn't do that. - Tim Post
22
[+19] [2009-04-30 12:15:48] Jamie Ide
try
{
// try something
}
catch (Exception ex)
{
throw ex;
}


I've seen this submitted in code samples from many applications. Typically the entire method is inside the try block.

(12) At least they throw the exception. A worse solution (that I've seen frequently) is to just swallow the exception and return like nothing happened. No logging or anything. Fun to debug. - Logicalmind
I see this one a lot. Usually, the origin lies in the debugger. Programmer wants to see what the exception is, but doesn't really want to handle it. So he puts the whole function in a try/catch, rethrows the exception, and puts a breakpoint on the 'throw' line, so that you can see what the problem is without really affecting the program flow. - GWLlosa
@GWLlosa -- That may be the thinking behind it sometimes but that makes no sense. In Visual Studio you can set the debugger to break on thrown exceptions for the same effect. Debug > Exceptions > check Thrown for CLR Exceptions. - Jamie Ide
(6) In C# you would want to just throw; When you do throw ex; it restarts the stacktrace in that method. You would have no idea where it came from. - David Basarab
Exception handball. - tomp
(1) I'm constantly debugging an app in which the previous developer's idea of exception handling was to put "//something didn't happen. return nothing and continue" in the catch. - Jon Ownbey
Or using exception blocks as quasi-goto statements. - Evan Plaice
(1) NO NO NO... take out the try/catch completely. Even 'throw' by itself loses thte line in this sub the error was on. You are NOT doing anything here. and BTW if you need a finally, and all you have in the catch is 'throw', syntax allows you to omit the catch clause. For a reason - you shouldn't use it unless you need it. Catch and log errors at the highest possible level and if you need to do something 'if error' and re-throw that can't be done in a finally, put the exception in an innerexception you throw to retain the full stacktrace. - FastAl
FastAl -- slow down and read the original question. No one is advocating this style of exception handling. - Jamie Ide
I have actually seen an empty try block with exception handling in the catch... - Chad
Here's why I do this (and forget to remove it when I check my code in): while I am writing code, I want to put a breakpoint on the line in the catch. - DOK
23
[+18] [2009-04-30 12:29:16] John Topley

Not only copying and pasting code, but copying and pasting code with comments and not updating the comments to reflect the new context!

(5) My favourite example : "/* Covert to uppercase */" <code that converts string to lower case.>" Further down was a method with the same comment, but it did upper case the string, so the programmer had obviously just copy and pasted then modified. No comments are preferable to wrong comments. - Bernard
(1) It gets real fun when people copy/paste your code that has your initials in it, modify it, and then when it doesn't work you get to deal with it. - Rorschach
@Bernard: If someone has to explicitly spell out that a function converts something to upper (or lower) case .. there are far deeper problems to worry about :) - Tim Post
24
[+18] [2008-10-26 04:23:56] Paul Nathan

Writing O(N!) code and passing it off as a working solution.

That irritated me.

(9) At least the guy has a working knowledge of recursion. - user9282
(8) At least it wasn't hyperfactorial. - TraumaPony
how big is N, 2? if so it doesn't matter :p but really some problems can only be solved in with O(N!) :( - hhafez
(1) If I recall correctly, N was in the 100K range. @.@ - Paul Nathan
25
[+16] [2008-10-26 01:46:21] Ryan

Unnecessary looping. For some reason, junior developers/programmers always loop more times than they have to, nest loops more deeply than they need to, or perform the same operation on a data structure twice, in two (or more) different loops.

(2) I once worked with a guy that claimed "all programming problems can be solved with two loops." The funny thing is that for the year we worked together, it was the right answer for the problems we faced. - Chris Lively
(5) haha. When I was working with VB6, one of the guys I worked with used to tell me "On Error Resume Next will fix all your problems! Just put it at the beginning of all your routines! You will never have to worry about exception handling!". - Ryan
ooh, wow. just, wow... - Erik Forbes
(1) well it would get rid of the error... how much data did he corrupt? - Matthew Whited
I hope you mean too many loops and not early loop termination... I'd say early termination is a sign of inexperience and foolhardy optimization. - A. Scagnelli
@Chris: in fact, all programming problems can be solved with one loop. This follows directly from the fact that the WHILE calculus is Turing complete, and from Kleene normal form. - Konrad Rudolph
@ Ryan ... that was so funny ...... got to admit I have done it many years ago ... when I started with VB6....... - captonssj
26

Wanting to "start over" on large projects whenever something in the existing framework doesn't match their world view.

(3) How about wanting to start over on a large project that was only large because of 3M+ lines of generated (and unused) code. - Matthew Whited
(2) Depends on the situation. If it is an existing and supported (commercial/open source) framework, this is indeed silly. If the framework in question is hacked together in-house and is more in the way of developing than actually helping the project, then throwing the framework out and starting over might even be the only right thing to do. ;-) - peSHIr
That also depends, peSHIr. No matter how hacked-together the framework in question is, if 3 million more lines of (usually similarly bad) code depend on it you can't throw it out and start over. You'll bankrupt your company in the meantime. "Slow and steady" refactoring is the best you can do in that case. - MadKeithV
(1) If you sit me down in front of something filled with living examples of every answer above this .. I'm either going to start expensing scotch or ask you to implement some kind of sanity :) That said, the kid who always knows best is probably the one that nobody but you had the misfortune of hiring :) - Tim Post
Slow and steady refactoring only works if there aren't senior devs who insist on keeping things just the way they are. Some people are content spending 3 days creating something that should take 3 minutes as long as they are getting paid, and they don't like learning anything new. - stinky472
@stinky472 - there are cases when the existing code is simply no longer maintainable (i.e. any change costs more than you could ever expect to gain back), but from my experience they are outnumbered 10 to 1 by rewrites that trade framework problems you know very well for framework problems you don't know well at all, with little to no actual benefit to the developers or end-users after the rewrite. - MadKeithV
27
[+13] [2008-10-26 02:14:50] Orion Edwards

This is the one I see most frequently by far

Imagine there is a method:

string GetConfigFromFile()
{
}


Now you need to use this method from another place. What do they do? THIS:

string GetConfigFromOtherFile()
{
}


This problem also extends to "intermediate" level programmers, who have learnt about things like function parameters, but will still do things like this:

int CompareNumbers( int a, int b )
{
return a.CompareTo(b);
}


.... and repeat again for floats, doubles, strings, etc, instead of just using an IComparable or a generic!

What about the void functions that return values? Those won't even pass the compiler. - Barry Brown
Good spotting. /me hastily edits - Orion Edwards
dont you mean Icomparable? or perhaps IEquatable .... - John Nicholas
err.. yeah, I do mean IComparable... Fixed - Orion Edwards
(1) +1. It's painfully obvious that the first 2 were written by an inexperienced programmer. Best answer I've seen so far. - Evan Plaice
(2) Intermediate programmers have only just learnt about parameters? - Dominic Bou-Samra
28
[+12] [2008-10-26 03:29:25] bryanbcook
public enum DayOfTheWeek
{
Monday = 1,
Tuesday = 2,
Wednesday = 3,
Thursday = 4,
Friday = 5,
Saturday = 6,
Sunday = 7
}

// somewhere else
public DayOfTheWeek ConvertToEnum(int dayOfWeek)
{
if (dayOfWeek == DayOfTheWeek.Monday)
{
return DayOfTheWeek.Monday;
}
else if (dayOfWeek == DayOfTheWeek.Tuesday)
{
return DayOfTheWeek.Tuesday;
}
else if (dayOfWeek == DayOfTheWeek.Wednesday)
{
return DayOfTheWeek.Wednesday;
}
else if (dayOfWeek == DayOfTheWeek.Thursday)
{
return DayOfTheWeek.Thursday;
}
else if (dayOfWeek == DayOfTheWeek.Friday)
{
return DayOfTheWeek.Friday;
}
else if (dayOfWeek == DayOfTheWeek.Saturday)
{
return DayOfTheWeek.Saturday;
}
else if (dayOfWeek == DayOfTheWeek.Sunday)
{
return DayOfTheWeek.Sunday;
}
}


when the following would have worked fine:

DayOfTheWeek dayOfWeek = (DayOfTheWeek)Enum.Parse(typeof(DayOfTheWeek), dayOfWeek.ToString());


Might want to fix the various typos in there - Joe Philllips
I used to do that when I just started out. :O - Quibblesome
Fixed my typos. btw - I've seen this appear in multiple code-bases. I'd understand why newbies would find the Enum.Parse call here confusing -- it reads even worse in VB.NET when you add the CType functions - bryanbcook
I do more in VB.Net that C#. This is a subtle one. I found a solution like this after thinking "surely there is a better way". - Anthony Potts
Still a teeny typo the Sundayy return. - Oddmund
Too bad we can't bookmark comments -- I didn't know of this one and would like to store it for later. - Nazadus
Fixed typo and formatting. - Michael Myers
Is not "(DayOfTheWeek)dayOfWeek" easier here? - tafa
@tafa you get a compiler error trying to explicitly cast the int to enum - Jimmy
you just don't need all these "else if" - Vimvq1987
29
[+12] [2008-10-26 05:23:42] bk1e

Using parallel arrays when an array of structures/records/objects would be more appropriate.

Comments that convey the author's uncertainty as to why the code works (e.g. /* I don't know why I have to frob the quux but if I don't then it crashes */). Note that these are sometimes also added by those who inherit the code later (e.g. /* TODO: Why in the world is this code frobbing the quux? */).

Comments copied from example code or containing boilerplate documentation.

Code "litter": unused local variables, member variables that are only used in one member function (and not saved across calls), superfluous blocks of code, etc.

(C++) Taking extra care not to delete NULL:

if(ptr) {
delete ptr;
}


(C++) Using unnecessary semicolons to avoid having to remember which blocks actually need them:

namespace foo {
int bar() {
...
};
};


(C++) Not even paying lip service to const correctness:

char* surprise = "Hello world!";


(Modifying a pointer to a string literal is undefined behavior, so this should be const char*.)

int add(int& a, int& b) { return a + b; }


(a and b must be lvalues even though they are not modified.)

class baz {
...
double get_result() { return m_result; }
...
};


(Calling get_result() requires a non-const baz.)

Um....deleting null in C++ can cause a run time error. - Whaledawg
I think that's his point - johnc
(12) No. The C++ language guarantees that delete p will do nothing if p is equal to NULL. - mafutrct
Borland C++ 4.5, if compiling for DOS target, really would crash if you did delete NULL; - Joshua
i love the sample TODO comment "Why in the world is this code frobbing the quux?" ... oh man. I have been writing almost this exact thing on code that i just inherited - que que
The const char bugs the hell out of me. Ostensibly because I care about correct code. Really, it's because gcc raises claxons about deprecated conversions from const char. (:P) - Bernard
Parallel arrays can make sense in Java. I've obtained a 30% speedup in one program that way. - finnw
I had a specific case where a co-worker insisted on avoiding a parallel array. Problem is that he completely neglected the notion that users are supposed to be able to associate per-element data externally and thought he could just stuff all the variables in one place. Sometimes the benefits of loose coupling outweigh utmost cache-line efficiency concerns. - stinky472
30
[+9] [2008-10-26 05:22:28] Todd White

I've seen a number of interns write this code in c#:

public type Property
{
get { return Property; }
set { Property = value; }
}


I actually crashed Visual Studio with code like this, because I typo'd the private member field "property" to the public member property "Property". - Jacob Krall
The people that write could like that, tends to debug the stack overflow exception too... - leppie
(6) BTW: Are non-programming related questions "stack overflow exceptions"? - EricSchaefer
(2) This actually happens to me from time to time, although it's either due to sleep deprivation or autocompletion, or both. ;) - steffenj
And that's why I only use backing fields when I need to work with the value inside the class. - Thedric Walker
I'm a big fan of output parameters, and you need backing fields to work with them. - FlySwat
31
[+9] [2008-11-28 18:42:42] Stephane Grenier

Another common simple one is:

if(value.equals("something"))
...


The problem with this one is what happens if value is null? Yup, a NullPointerException. Happens all the time. Instead it should be:

if("something".equals(value))
...


Reversing the order can never give you a NullPointerException.

(19) -1: this may protect you from NPEs but it hurts readability. I don't think avoiding this technique is a mark of inexperience. - Simon Nickerson
(16) It also depends whether value is allowed to be null. If it shouldn't ever be null, the first version will at least fail early with an exception. The second version will hide the bug. - Dan Dyer
If you don't do this, you'll have null checks everywhere (ie. you'll have to care if it can be null or not). And this also avoid trying to find those really hard to find NPE's. - Stephane Grenier
(3) I subscribe to the constant values on the left hand side, it might hurt readability slightly, but it prevents the occurrence of unintentional assignment in place of evaluation, the same as if(myvar = null) { } will compile and potentially run causing a hard to find bug, but if (null = myvar) { } will cause an easy to find compilation error. - BenAlabaster
(1) This does not hurt readability in the slightest - the only problem with it really is that the value should be a constant, not a string literal. That way, it is really easy to detect what is trying to be matched (Java syntax): public static final String THIS_THING = "this that and not the other"; // ... other code if(THIS_THING.equals(value)) { // ... etc } - MetroidFan2002
learned something today - Isaac Waller
These sorts of things irk me. value.equals("blah") and "blah".equals(value) ought to be identical -- in the same way that a==b and b==a are identical -- but they are not. Try to explain it to a beginner. They'll end up internalizing it as a syntax rule and "black magic" rather than understanding what's going on. - Barry Brown
(8) i would rather go for string.Equals(stringA, stringB) ... that's the most safe technique! - Andreas Niedermair
(1) I'd rather check for the NPE than write something like this. Its really hurts the readability - O. Askari
(1) Ok, so when playing Russian Roulette, its safer to spin the chamber in the OTHER direction? - Tim Post
32
[+9] [2009-07-13 16:11:49] Dinah

2 words: arrow code

For those not familiar with the term:

if () {
if() {
if() {
if() {
// notice the shape of all the nesting
// starting to resemble an arrow
}
else {}
}
else {}
}
else {}
}
else {}


I actually saw someone use 4 if() statements for 1 variable in this arrow formation. The first checked to see if the variable existed, the second was wether it was null, and if it wasn't, it checked to see if it had the right string. Then, a further nested if() verified if the variable was the same as another variable, and then executed a while loop. I almost cried reading it. - MT.
33
[+8] [2009-05-28 06:45:23] too much php

Hand-built Date/Time Functions. Usually when a programmer shows me some of his old code (written when he was just starting in programming), there are at least one or two functions to add/subtract dates, or get the total number of days in a given month (e.g., 28 for February). Experienced programmers have learned that dates are actually very complicated, and so they use their language's built-in date/time libraries so that they don't have to deal with time zones, leap years, leap seconds, daylite savings, etc, etc.

34
[+8] [2009-05-20 21:55:13] jfar

In a static language using switch statements all over the place when inheritance will solve your problem. Example is simple but I hope illustrates the point.

class Car
{
public string Name { get; set; }
public void Drive( int speed ) {}
}

var myCar = new Car();

switch ( myCar.Name )
{
case "Mustang":
myCar.Drive(120);
case "Corolla":
myCar.Drive(60);
}


Vs.

public abstract class Car
{
public abstract Drive();
}

public class Mustang : Car
{
public override void Drive()
{
//go fast
}
}

public class Corolla : Car
{
public override void Drive()
{
//go slow
}
}

var myCar = new Mustang();
myCar.Drive()


35
[+7] [2009-04-30 23:01:34] Barry Brown

You want to know if an integer x is between 0 and 100? Do it this way, of course:

for (int i = 0; i <= 100; i++) {
if (x == i) {
System.out.println(x + " is in range"); // or something similar
}
}


I found something like this inside another loop whose purpose was to determine which elements of an integer array were between 0 and 100.

wow.... that is absurd - Gavin H
How do they tell if a number is 'out of range'? I presume 'else' is not allowed. - Fantabulum
36
[+6] [2009-05-20 16:43:02] community_owned

This is great but it would be really helpful to see the proper ways to write the code and why. Not everyone has years of experience :)

37
[+6] [2008-10-26 02:22:15] FlySwat
public void DoSomething (bool DontDoSomethingElse)
{
}

// Later

DoSomething (!DoSomethingElse);


(1) I've done that countless times. And sometimes I get so mad at what the customer requirements are that I still do it, just to hack something up and make them shut up. Later, I always regret it. - Tom
(2) I don't get it, any specific example please? - sundar
(1) passing a "Don't do this action" boolean, instead of passing a "Do this if true" boolean, can cause really confusing code down the road. - FlySwat
Actually, I've done this before when working with enabling/disabling controls on a form or setting the visibility of controls. In that context it can make sense, but it is a design pattern that has limited usefulness. - Rob Z
38
[+5] [2009-05-21 10:26:20] Alterlife

int i; // define i as an integer 

(11) int i; //define i as integer //comment the fact that i is being defined as an integer - JohnFx
(5) Hey, at least it's not in Hungarian notation. - Barry Brown
39
[+4] [2009-05-21 03:40:20] David Plumpton

Re-implementing library functions without realizing it.

(2) Well that depends on the library/framework... then again that is why we have Bing and Google - Matthew Whited
Sometimes, reimplementing library functionality is not bad. Suppose you just need a small function out of a huge library that does everything and the kitchen sink. You have either two choices: 1) reimplement the functionality in your code or 2) having a dependency towards this library. Unless I need a lot more out of the library, I would choose 1. - Stefano Borini
40
[+4] [2009-05-21 07:16:51] James

Beyond the obvious of rolling your own solution to common problems with common solutions...

a = 3; a = 3; // Sometimes the first set doesn't seem to work.

Or other forms of superstitious programming. You really only see such when the person writing it doesn't undestand what they're doing.

Though I swear, while in college, I once made something in C compile by adding the line:

short bus; // This program rides the short bus.

I kid you not. (and no, there was no reference to 'bus' in the program. To this day, I'm not sure how it fixed the issue, but I was surely a noob at the time.)

+1'd for the funny, plus I had this vision of a new C programmer typing random phrases into the program to get a successful compile...Of course I know nothing of C, so 'short bus' may be a very relevant non-random compile-ensuring prayer-string. Still funny to me, though =) - David Thomas
(1) Probably had to do with memory overwriting or alignment. - korona
41
[+4] [2009-05-20 19:54:20] community_owned

Coding by superstition.

42
[+4] [2009-05-20 17:10:16] Jeff Thompson

If you think O(n) is more flexible than O(1) because it has a variable, you are inexperienced.

Common and Real mistakes:

• Not commenting.
• Not cleaning up code/interfaces after getting a system to work.
• Not communicating with their customer (internal or external)
• Not taking the time to understand the systems they are interfacing with.
• Modifying another system's internals with a hack to support work on another system.
• Not verifying that their code compiles and that the app successfully launches before checking in.
• Giving best case estimates that only include implementation time.

(2) I would also add that assuming a o(1) solution is always faster than an o(n) solution is also a newbie mistake. - Jay Dubya
43
[+4] [2009-04-30 12:33:13] John Topley

Not realizing that the toString method exists, instead doing something like:

Person p = getPerson();
LOG.debug("Got person, first name is " + p.getFirstName() + ", surname is "
+ p.getSurname() + ", age is " + p.getAge(), " gender is "
+ p.getGender());


What if it's a library class and toString() doesn't give useful info? - finnw
(1) Then it's acceptable to get useful info out any way that you can. For the examples I was referring to the toString method could have been overridden. - John Topley
44
[+4] [2008-10-26 08:30:31] user9282

Using constants in code and wildly hunting for them whenever they need to be changed.

45
[+4] [2008-10-27 03:07:26] Terry Donaghe

Young coders are often very enthusiastic and rush headlong into solving problems without a care towards reuse, coding standards, readability, testing, or anything other than just "gettin' r done."

Another newbie habit, especially from guys who've really boned up on patterns and object oriented programming, is over-design. Creating a beautiful class hierarchy that looks fantastic in UML but ends up being a maintenance nightmare - too complex to easily understand how the code flows from top to bottom, no regard at all for performance, etc.

46
[+4] [2009-07-13 12:40:28] TM.
try {
// some stuff
} catch (Exception e) {
// should never happen!
}


You shouldn't throw away an exception without logging or anything, even if you think it will never happen! It's made worse when catching any type of exception.

could you elaborate? Are you saying one should never catch exceptions in a method, or that one should catch specific exceptions? Personally, I catch specific exceptions when I know they're a possibility, but have this code structure if I know I can recover in the parent method. - Steve
(1) I'm saying that one should never just catch an exception and throw it away with no logging or anything... even if you think it will "never" happen. - TM.
+1 I actually had the exact same statement in some of my code. It was there to stop a compiler warning... Interestingly, one day I was running through that section of code and I saw it throw an exception. The situation was quickly, and properly, rectified. - Chris Lively
47
[+4] [2009-10-09 16:15:37] John Isaacks

When someone spends hours repeating a task when they could take 5 minutes to write a script to do it for them and never have to do it again.

(1) And the flip side of that. Someone who spends days writing a script to automate a task that they could complete in 30 minutes and only have to do a few times a year. - JohnFx
48
[+4] [2010-07-13 16:07:18] Ed B

When your UI 'developer' takes way too long on the layout for an intranet site, delaying the release and costing the company time and money because they couldn't process users...because he heard from some blog that html tables are bad.

Other developers couldn't figure out the messy div and css hell to modify the complicated layout, so they rewrote it using tables and css & everyone was happy again

(1) +1 on the tables statement. I think you could go further and say, "An inexperienced web programmer is one who will ask on SO how to get two (or three) div's to automatically expand to the same height together! And insist it has to be done in css because everyone knows tables aren't for layout. Oh to be stupid again. ;) - Chris Lively
49
[+3] [2009-07-13 13:37:37] hythlodayr
1. Fragile logic.
2. No abstractions with humongous code--if I'm hitting page-down more than a few times...
3. Engineering code for "the future".
4. Abstracting unnecessarily.
5. Extension of #3 & #4 for OO: Huge # of classes in the first pass of a design, when a handful is what's really needed.
6. Coding without really understanding the user requirements.

To be honest, even experienced coders--though perhaps barring the godly ones--are guilty of all of these but I think the scale of these mistakes set experienced and inexperienced coders apart.

I also think #6 is the hardest to get "right" & the guy that can massage out the necessary user requirements isn't necessarily the best programmer either. In theory, a good business analyst--if you have one handy--can capture the correct requirements. In practice, the programmer needs to understand the business well enough to notice oversights in design and tease out unspoken assumptions on the business side.

50
[+3] [2009-06-24 21:35:11] jprete

Doing shotgun-style modifications to an existing codebase in order to get something running without paying attention to how those changes affect the rest of the system.

51
[+3] [2009-06-24 21:47:28] rein

Wanting to rewrite every piece of code they aquire. This is a sheer sign of newbieness.

52
[+3] [2008-10-26 19:13:03] dpurrington

Gratuitous usage of reflection.

(4) I doubt inexperienced programmers are able to use reflection at all. - mafutrct
@mafutrct are you suggesting that it is hard to use? I think it's pretty straightforward... - TM.
@TM I'd say its not difficult to use as such, but that concept takes some getting used to, I'd only expect to see a newbie using this having picked it up inadvertently from a piece of sample code somewhere. - TygerKrash
Oh no, it's hot stuff, and the best way to call any method. Never just call a method. Use Reflection to call it. LOL - DOK
I've seen plenty of it. Note the question is regarding inexperienced programmers, not incapable programmers. There are a lot of solutions that work at small scale (i.e. college project) that break down at industrial scale. - dpurrington
53
[+3] [2009-05-20 16:37:38] 48klocs

Comments in code telling what you're doing rather than explaining why you're doing it is a dead giveaway. I look at it and think to myself "wow, they were really struggling to piece together how the thing worked."

We're ostensibly professionals. I don't think we need any comments to explain what's going to happen in that foreach loop. Less of that, more explaining why you're doing something that isn't immediately obvious (OK, I see you're checking the return code against a magic number - why?).

(5) I respectfully disagree. If you have 20-30 lines of code representing 3 logical stages in a process, I find comments are a good way to break up the code so it's easier to apprehend. Even though anyone can tell the difference between and appetizer and an entree, headings on a menu help with that. I leave the "explaining why" for cases when I think the way something is being done is non-obvious or works around a subtle bug. - Nick
54
[+3] [2009-05-20 18:44:10] Jeremy Murray

Coding newbie mistakes:

• Code compiles but doesn't run.
• Code runs but fails unit tests.
• Breaking published style guidelines.
• Changing line-endings, even mid-file.
• Not commenting their code as they write it.

Coding group newbie mistakes:

• Not asking for code review throughout their first projects.
• Not documenting specifications and deliverables for their projects.
• Not Googling before asking questions.
• Not spending time familiarizing themselves with their daily toolset.
• Throwing their two cents into every group discussion.

55
[+3] [2009-05-20 19:01:20] Dana Holt

A few I've seen:

• Writing single methods/functions that do several unrelated things
• Rewriting functionality that is already available
• Fixing bug symptoms instead of the root cause

56
[+3] [2009-05-20 20:02:49] Paulo Guedes

I once came accross a code like this:

If month="Jan" Then
Responde.Write "January"
Responde.Write "Sun Mon Tue Wed Thu Fri Sat"
For i = 1 to 7
Responde.Write i & " "
Next
For i = 8 to 14
Responde.Write i & " "
Next
For i = 15 to 21
Responde.Write i & " "
Next
For i = 22 to 28
Responde.Write i & " "
Next
For i = 29 to 31
Responde.Write i & " "
Next

ElseIf month="Feb" Then
Response.Write "February"
Responde.Write "Sun Mon Tue Wed Thu Fri Sat"
For i = 1 to 7
Responde.Write i & " "
End
For i = 8 to 14
Responde.Write i & " "
Next
For i = 15 to 21
Responde.Write i & " "
Next
For i = 22 to 28
Responde.Write i & " "
Next
For i = 29 to 31
Responde.Write i & " "
Next

ElseIf month="Mar" Then
Responde.Write "Mars"
Responde.Write "Sun Mon Tue Wed Thu Fri Sat"
For i = 1 to 7
Responde.Write i & " "
Next
For i = 8 to 14
Responde.Write i & " "
Next
For i = 15 to 21
Responde.Write i & " "
Next
For i = 22 to 28
Responde.Write i & " "
Next
For i = 29 to 31
Responde.Write i & " "
Next
... and so it goes.


Not only the guy didn't know anything about arrays and loops but he also lacks experience about how different are the months within a year. :-)

57
[+3] [2009-05-21 14:59:00] sjh

things that boil down to:

if some_bool == true:
some_bool = false
else:
some_bool = true


some_bool = !some_bool


and for-case [1] structures.

Agree on the first part (fundamental lack of knowledge of how booleans and the language in general work), but the for-case is eerily similar to a widely-accepted way to handle a state machine in an interrupt, like when generating an arbitrary wave form over several successive interrupt requests. - David Lively
58
[+2] [2009-05-20 18:50:48] Kevin

for (int i = 0; i < 12; i++)
{
if(test)
{
i=12;
}
else
{
//do stuff
}
}


59
[+2] [2009-05-20 17:27:50] Alexander Kahoun

Usually when you see something like this:

public static string ProcessUpdate(string xml)
{
string result = "";
try
{
//code...
}
catch (Exception exception)
{
result = exception.Message.ToString();
}

if (result == "")
result = "True";

return result;
}


well.. that is better than an architect I know that wrapped damn near everything with try {doWork();} catch{} .... If you are gona eat the exception at least be kind enough to do a Debug.Write(ex.Message); ... still have me having to step your 10k+ lines of generated B.S. - Matthew Whited
60
[+2] [2009-05-20 16:30:01] Michael Luton

Using the ternary operator at every available opportunity. Especially when the ternary operator runs really long and an if/then/else statement would be more readable.

$foo = (count($articles) < $min_article_count) ? get_articles_by_genre($status, $author,$site_id, $genre) : get_hot_topics($board_id, $platform_id,$min_date);


versus

if (count($articles) <$min_article_count) {
$foo = get_articles_by_genre($status, $author,$site_id, $genre); } else {$foo = get_hot_topics($board_id,$platform_id, $min_date); }  Actually, I'm of opposite belief; I think newbies tend to overuse if pattern. Ternary operator is better. It forces foo to be defined in one expression. Thus it could be marked as immutable. Using if this way is not good, you could accidentally do something about the other branch of if and compiler wouldn't notice. If the conditional expression is complicated, it should be put into its own function, or calculated before the definition of foo conditonal = count($articles) < $min_article_count$foo = conditional ? ... : ... - egaga
Formatting that as "$foo = (condition) ? \n (true-expr) \n : (false-expr)" would help. I don't mind the conditional operator, although get a both nervouse when they start getting nested (a?b?c:d:e etc). - araqnid (2) I'm with both Michael Lutton and egaga on this one. I think (a) that inexperienced programmers almost never use the ternary operator but (b) that your example code is clearer without it. I tend to find that inexperienced developers, on the contrary, don't use the ternary operator in situations where it would be clear and concise. - Gregory Higley What about never use the ? operator. - Guillaume Massé Would it be ok to, in his example, use the ternary operator with multi-line formatting? There is no rule that says it needs to be on one line. Though, I'd probably say screw it and use if statements because trying to figure out if the former is more acceptable takes too much effort and doesn't really make any different in the long run. - user606723 Newbs often try to clever when they do things. Often this leads to code that is technically better in some terms, but is worse in the terms that really matter- readability and maintainability. - user606723 61 [+2] [2009-06-24 22:32:44] JohnFx Using input/output parameter types that are way too general and require the caller to understand the innards of the methods to use it and force tight coupling. SqlDataReader getEmployee(int EmployeeID) {....} void addInvoiceLineItems(object[] LineItems) {....}  GetEmployee(int EmployeeId) seems pretty clear to me... - David Lively @David Lively - Think so? So what columns will be available in the SQLDataReader returned by that function? - JohnFx 62 [+2] [2009-07-13 11:20:46] LKM Inexperienced programmers typically don't know the Libraries well, so re-implementation of common library functions (say, to parse dates, or escape HTML) is often a good way to tell how much experience somebody has. I kinda disagree. The're a lot of libraries with a lot of functions. Knowing all the functionality is a lot of work. You create your own implementation and later on when you find someone that is using a library function, you will use that one instead :) - PoweRoy That's why only people with experience know about all those libraries :-) I agree, though, that you can't expect people to know about all libraries, but I think knowledge of some common libraries is a good indicator of experience. - LKM I find a lot of experienced people tend to hold on to those personally developed libraries far too long. Not too long ago I was on a project where we were forced to include one giant library to do simple things like string parsing. The team was actually encouraged to add new methods to the library instead of using what was already provided in the framework. Very weird. - Chris Lively 63 [+2] [2009-07-13 12:39:51] Slace In web development not understanding the difference between the client and server is something that I've seen quite a few times from new developers. I've been asked why this didn't work plenty of times (ie - why my doesn't my alert show): Page.ClientScript.RegisterStartupScript(this.GetType(), "notification", "alert('Success!');", true); Response.Redirect("/default.aspx");  (I think that code's right :P). And using alerts for debugging JavaScript, man that is such a frustrating thing to see, particularly when using Firebug! (1) Alerts used to be about the only way to do it. Maybe that's a sign of a "too experienced" dev? ;) - Chris Lively 64 [+2] [2009-07-13 16:03:03] DroidIn.net 1. Not using code conventions, for example using first-uppercase to name you variables in Java (insert you favorite language here) 2. Methods that go on and on and on 3. Everything is in one class 4. Copy/paste code 5. Nested loops 6. Mad chaining (darn, what did just throw that NullPointerException?) 7. Exception swallowing 8. Commented out code 9. System.out.println 65 [+2] [2010-07-13 16:13:04] jasonmw calling GC.Collect() regularly to "Clean Up" 66 [+2] [2010-07-13 16:25:08] TheJuice Other posts have highlighted 'lots of comments' as a sign of a novice programmer but I would refine that to superfluous comments e.g. // if there are some things to process if (things.size() > 0) { // loop over the elements of things for (int i=0; i<things.size(); i++) { // sparklify each element things[i] = sparklify(things[i]); } }  (1) +1 As an interesting aside, I almost always put a comment on the same line as the end brace to identify the section it's in. When you have multiple nested braces it helps when trying to find the bottom of the last one. Now, putting them at the top.. waste of space. - Chris Lively 67 [+1] [2009-08-08 17:24:20] df. @Override public int hashCode() { return 0; }  68 [+1] [2009-07-14 06:07:27] Kthevar Most of ASP.Net newbies try to frame the HTML inside the aspx.CS file instead of aspx file. If you hard code the HTML code inside the .CS file there is noway the designer can make the changes without developer support. The code is no longer stable. Eg: Literal lt=new Literal(); lt.Text=" test....."; This is what happens when PHP guys accidentally get a real job. - David Lively 69 [+1] [2009-07-22 16:49:44] Jeffrey Kemp Using a word processor to write code. More often than you'd think I've had someone ask me why their code doesn't compile, and it's because they've got some magic quote characters' instead of just ' or ". Way back when I was a co-op student, I worked on a government project where it was mandated in the contract that we use MultiMate for editing code. Shudder. Then again, what kind of company puts co-op students on an air traffic control system? - James McLeod Until I migrated to Win7-64 and couldn't use it except in a virtual machine, I used PC-Write 3.0 as a code editor for many projects. It ran fast in 1986 on a 4.77MHz PC, and even faster on more modern hardware, and had some nice features that Visual Studio lacked. Unlike many word processors, it saved files in normal text format. - supercat 70 [+1] [2009-10-17 04:20:15] Scott S. I was always fond of if (x = 1) { ... }  But maybe that is more inexperienced than you were thinking. 71 [+1] [2010-07-13 13:29:30] Cipi Just to name a few... 1) In C# I love when I see this: int size = 0; public int Size { get { return size; } set { size = value; } }  And throughout the code only size is used, never Size. You could only write public int Size{get;set;}. 2) In C++ a common mistake is not knowing that * refers to "identifier" and not "identifier's data type", so when there's a need to have 2 pointers of the same data type: int* a, b;  Only a is a pointer to an integer, and b is just another int, not a pointer. 3) Not knowing about the breakpoints... so the code looks like this: int a, b, c; decimal d; ulong l; //Console.WriteLine("a = "+a); a = (b++)*c+99+a; //Console.WriteLine("a2 = "+a); //Console.WriteLine("c = "+c); //Console.WriteLine("a dec = "+((decimal)a)); d = ((decimal)a) + (--l); //Console.WriteLine("d = "+d); //Console.WriteLine("l = "+l);  4) Not using threads when there is an obvious need for them. 5) Flushing then closing a stream. stream.Flush(); stream.Close();  6) Emoticons and self glorifying comments (I hate it, just write the damn code...): foreach(var i in List) { //i.remove() - oops, list in foreach cannot be modified, this would raise error =)) =P ;) i.writeOut(); }  7) Assigning 0 to integers in more then 1 line: int a,b,c,d,e,f,g; a = 0; b = 0; c = 0; d = 0; e = 0; f = 0; g = 0;  8) Ignoring the existence of ternary operator: int a = 10; int b = 1; if(b < 2) { a = a-b; } else { a = a+b; } //a = (b<2?a-b:a+b);  I've seen people replace ternary operators with if..else blocks because they didn't understand what it did. See the jQuery source for many examples. - David Lively 72 [+1] [2010-07-13 15:23:54] Niels Basjes Thinking that Duff's device [1] is a neat feature of the C programming language they can use to improve production code. Quote from Wikipedia: send(to, from, count) register short *to, *from; register count; { register n=(count+7)/8; switch(count%8){ case 0: do{ *to = *from++; case 7: *to = *from++; case 6: *to = *from++; case 5: *to = *from++; case 4: *to = *from++; case 3: *to = *from++; case 2: *to = *from++; case 1: *to = *from++; }while(--n>0); } }  [1] http://en.wikipedia.org/wiki/Duffs_device 73 [+1] [2009-07-13 12:08:57] mandrake • Adding using directives and declarations in header files. • Making class internals public instead of adding accessors. • Always passing by value instead of const reference. • Not implementing (or hiding) copy-constructor and assignment operator for objects that allocates and handles memory. • Having method names longer than the method. Actual example (!): dontResendSigIntInfoIfReasonAllreadyExistsWithinTimePeriod(...)  74 [+1] [2009-05-20 20:16:45] community_owned Taking comparison to constants too far. This is understandable: if(5 == x) { /* something */ }  but this is taking it too far if(5 < x) { /* something */ }  especially if there are complex conditions involved. huh? why is "if x is greater than 5 do something" taking it too far? This seems like a very valid constraint that anyone would run into. - Steve @Steve: Looking at the code, can you tell what 5 represents? Named constants are better. - Nikhil Chelliah (1) @Steve, It is not whether the constraint is valid. It's the direction of the comparison that's been flipped. Consider that the constraint that you need to enforce is that x is greater than 5. Most of us would write "if(x > 5)". We do this even if we write "if (5 == x)" rather than "if(x == 5)" because we don't want an inadvertent omission of '=' sign in the latter expression change the meaning from comparison to assignment. Now taking this a bit further, we tell ourselves, let's be consistent and start writing all comparisons this way. So our code is now sprinkled, or shall I say littered with - community_owned (contd) statements such as if(MY_CONSTANT < (x*(expr)/y*expr1)) rather than if( (x*(expr)/y*expr1) > MY_CONSTANT). Most of us are used to the former though on occasion we may resort to the latter if it helps clarity. - community_owned 75 [+1] [2009-06-24 21:01:42] João Marcus Inexperienced developers usually rant a lot about everybody else's code. They're not bad coders, they're just not used to dealing with rotten code everyone usually finds in real life. They still don't have the experience to understand the context behind the code. Was it caused by last-minute requirement changes? Was it caused by real sloppy coding? Was it caused by Dr. Jekyll requirements (looks fine at first but grows up to be a real monster)? I find rotten code all the time, I've been coding for a good number of years, and I still grumble about it when I see bad code :) - hythlodayr 76 [+1] [2009-06-24 20:31:09] Jim Evans Multiple nested regions. (.Net) 77 [+1] [2009-05-20 16:52:58] egaga Using comments for a piece of code that should be put into a separate method. x = ... y = ... // foo as a bar return x*y+35  this should be instead: return fooAsABar(x, y)  78 [+1] [2009-04-30 23:09:43] NotDan //Add value to i i += value; //Check if i is less than 10 if(i < 10) { //if i is less than 10, return true return true; } //otherwise i is greater than 10 else { return false; }  (2) yes, that last comment is totally wrong. It should be "otherwise i is greater than or equal to 10"! :) - dotjoe 79 [+1] [2009-04-30 12:09:51] Alberto Zaccagni I still sometimes print out information to console when I should have used a logger or entered in debugging mode; so using this: System.out.println("Reached foo init, bar is: " + bar);  ...is risky because it could end up in production environment. 80 [+1] [2009-04-30 12:11:03] leppie Using a Verdana font to program in.... (1) Having never done this, could you explain this one further? - Sukasa As opposed to something like Courier? - Tom Savage 81 [+1] [2008-10-31 15:14:35] Tor Haugen string myVariable; ... Foo(myVariable.ToString());  82 [+1] [2009-05-20 19:23:11] BZ. Putting anything in the comments/log/Error statements that they wouldn't want published. I.e. Errors that use one of George Carlin's 7 words Log Statements that would be bad if pushed to production 83 [+1] [2009-05-21 15:35:53] community_owned Inexperienced software designers often attempt to use the Observer Pattern in multi-threaded software without carefully considering deadlocks and race conditions. 84 [+1] [2008-10-26 08:29:16] community_owned In the following, "files" is a very large array of strings. This was also a design decision made by the programmer in questions. private String findThePlaylist(String playlistFileName) { String theone = ""; for (int i = 0; i < files.length; i++) { if (files[i] == playlistFileName) { theone = files[i]; } } return theone; }  85 [+1] [2008-10-26 00:28:00] EvilTeach if ((strcmp(command, "Q")) == 0) { // do stuff }  instead of if (command[0] == 'Q') { // Do stuff }  can cost a function call, instead of a single machine instruction. What's the point here? Do you mean they should be using strncmp or similar, or just that they should 'return strncmp(command, "Q");' ? - Orion Edwards (1) I think the point is to avoid an unnecessary function call when comparing a single character: "if (command[0] == 'Q') return 0;" Strings are arrays, and single-quoted literals are chars! - Sherm Pendley This can be useful at times. This could occur as a part of larger function, and in that case, returning 0 instead of returning strcmp() result can be more desirable. Also, comparing command to "Q" compares 2 characters, not 1, and is more extensible than doing it manually. - community_owned The point here is that there are at least three things potentially wrong with that line of code. - moffdub Another problem is not using strncmp() instead. - Sherm Pendley Ya, I find the use of a function call for what ought to be a single character compare, ummm. objectionable. Single char compares could potientially compile to one machine instruction. - EvilTeach (6) Comparing function call to "single machine instruction" doesn't matter except in the cases where it actually does. The real problem is that it's not as clear. Also, your two examples aren't the same. The first only accepts the string "Q", while the second accepts any command that begins with the letter 'Q'. - Andy Lester 86 [0] [2008-10-26 00:36:21] Jedidja Not having any unit tests for their code. Or (perhaps, even worse) having """unit tests""" for their code that don't really test anything / test a hundred things at once. 87 [0] [2008-10-26 19:33:22] Quibblesome • Exposing collections as get; AND set; • Code in the file that doesn't actually do anything but was included in a bunch of changes they implemented to get it working meaning they think the pointless code "somehow" helps. I like the first bullet, but would generalize it as weakly typed object properties. More often (on .NET) I see methods that return or take as parameters DataSets or DataReaders. - JohnFx I mostly agree with the first. But when I don't want to implement custom serialization code I sometimes learn to deal. - Matthew Whited 88 [0] [2009-05-20 20:05:00] Nils-Petter Nilsen I came across this one a while ago, in some code I inherited from a programmer that simply wasn't able to gather experience, even after several years in the job: String dir = "c:/foo"; for (int i = 0 ; i < 2 ; i++) { //Do stuff in folder dir = "c:\bar"; }  I've also met 2nd year programming students that simply couldn't grasp the concept of for loops. There's a giveaway... could it be that "dir" was being modified in the "//Do stuff in folder" section you commented out? And said programmer was simply resetting the value at the end? - mxmissile (1) Valid point, but it was used exclusively inside the loop as source folder to copy files from foo and bar to one target folder. I'd make a copyFrom(String dir) method, or something to that effect, and call it twice with foo and bar as parameters. - Nils-Petter Nilsen Must escape slash in dir = "c:\bar"; or dir = "c:/bar";. - Cipi 89 [0] [2009-05-21 02:50:42] community_owned In C; #ifndef TRUE #define TRUE 0 #endif #ifndef FALSE #define FALSE 1 #endif  You have no idea how easy it is to miss what is wrong with this code when it's buried amongst a bunch of other code, and how hard it is to track down the problems that it causes. 90 [0] [2009-05-20 19:28:40] ziggystar I've seen the following (or similar) code written both by my current colleague and our predecessor. some_string[strlen(some_string)] = 0;  91 [0] [2009-05-20 18:23:34] Decio Lira I just saw this move(0+shift);  92 [0] [2008-11-16 14:57:16] Nazadus I saw this once: object obj= sdr["some_int_value"].ToString(); int i = 0; try { i = (int)obj; } catch { }  Int32.TryParse, anyone? • Using try/catch for converting data. • Thinking try/catch has zero penalty. Even simply wrapping it in a try has a cost, albeit not much. It doesn't take too many exceptions to make that an expensive call in loops. • Using try/catch on every other line of code. All of which has nothing in the catch block. • Not commenting what a method/function does. This is a personal pet peeve of mine because I've seen someone have two methods with one having a '2' at the end... and both have very subtle changes. • Failing to understand why using the 'using' keyword is important when dealing with connections and datareaders. One can do without it, yes, however the using forces you to close the connection. I've yet to find a reason why not to do it. • Not understanding what transactions and stored procedures are important. Inconsistant data, anyone? • Not understanding why constraints are important. Race conditions, anyone? Int32.TryParse was introduced in .NET 2.0. Some of us have been coding longer than that. So your Int32.TryParse comment is invalid. - mxmissile (1) My argument is only invalid if we're using .NET 1.1 or older. We weren't. In fact this particular person argued for .NET 3.5. He also argued that catching something had minimal cost. By eliminating the try/catch we significantly gained in speed because we had a 40% catch rate. This along made something go from 23 hours down to 12 hours. (not just a single try/catch, we had many of them that added up). - Nazadus 93 [0] [2008-10-27 13:24:30] Thedric Walker I don't know but this seems a tad bit suspicious to me: foreach (BaseType b in collBaseType) { if((Type)b.GetType()).BaseType == typeof(DerivedType)) this.saveColl.Add(c) }  where b has a type derived from DerivedType. 94 [0] [2009-04-30 23:07:28] NotDan if(something) { } else { doSomething(); }  Nothing wrong with that, means the person knows how compilers work and is trying to help the compiler. Means they learned assembler first. Can be used for other readability issues that reduce human error. - dwelch (3) "trying to help the compiler" should be an entry of its own on this list. Ugh. - JohnFx 95 [0] [2009-05-28 08:38:57] Patrick Gryciuk If code segfaults printf("HERE"); do_something(); printf("HERE"); do_something2(); printf("HERE"); do_something3(); printf("HERE"); do_something4(); printf("HERE");  and counting how many "HERE"s there are before the code segfaults. (2) Yeah, everyone knows the "professional" way to do it is to use "Here1", "Here2", "here3", etc. =) - JohnFx I tend to make an "echo 'Hi!';" statement, and then cut+paste it into different places of the script. This way, I still only get one 'Hi', and I know where it is leaking. (I need to find a more efficient method) - Chacha102 (2) I do this with PHP because... well, it's already PHP - Carson Myers 96 [0] [2009-06-24 20:27:20] Jin Kim On the subject of exception handling: 1. Swallowing an exception and doing nothing with it. (If nothing is done, it should be passed up the call stack) 2. Swallowing a custom exception and throwing a more generic exception. 3. Throwing a custom exception as a result of a generic exception being thrown, but not chaining the custom exception to the originally thrown exception. (1) There is an excellent case for #1 if you are in the threadpool, or your function might be called from the threadpool. When I'm using the threadpool, I generally wrap the passed function in a closure so that I swallow the exception (after logging it). Otherwise you bring down everything when the exception is thrown. - Steve I'd agree with 2 & 3, but not 1. There are times when you want to swallow specific exceptions, but they are usually very few and far between. - ChrisF 97 [0] [2009-07-13 11:01:24] bobobobo Conversions from wide character type to ascii when unnecessary 98 [0] [2009-07-13 12:30:58] SilentGhost irrational wishes (of this sort [1]) without regard for readability, maintainability, etc. [1] http://stackoverflow.com/questions/1118994/php-extracting-a-property-from-an-array-of-objects 99 [0] [2009-07-13 15:54:09] Kelly French Not understanding the concept a = a + 1; When I was a lab assistant in school, I guy came for help with an intro to Fortran assignment and couldn't even program a loop to increment a simple int variable with a = a + 1;. When I refused to write the code for him (after 10-20 minutes of trying to explain the concept) he then declares that I'm an idiot and he knows what he's talking about because he's taken the intro to Fortran class three(!) times. You might say that this wouldn't happen in the real world but I worked with a guy who 'taught himself' to code by supporting some obscure database product. He barely understood the code of the import routines. When our manager forced him to write a program in 'C' (after being to the training class) he would come by for help with the same basic loop/a=a+1 type problem. Needless to say, he didn't pass the test. Sigh. 100 [0] [2009-07-13 16:01:18] Martin Wickman The biggest giveaway is definitely programmers using public static methods all over the place. That is, knowingly or (hopefully) unknowingly using OO features as an excuse for writing procedural code. 101 [0] [2009-07-13 13:10:05] CoderTao I'd argue that terrible variable naming is one of the best giveaways (along with poor structure). The worst would be two-three letter names for class variables. 102 [0] [2010-07-13 15:33:51] reece Using idioms from one language in another (they indicate a programmer inexperienced in the given language). Java in C++: File * file = new File(argv[1]); DoSomethingWithFile(file); // no 'delete file;' here -- should use a smart pointer / RAII  C++ in Java: public SomeFunction() { FunctionTracer trace = new FunctionTracer("SomeFunction()"); // oops -- trace does not get cleaned up here. }  or: public Something() { File f = new File("test.txt"); // ... // no f.close(); -- should use a try ... finally block (using in C#) }  BASIC in C/C++: #define BEGIN { #define END } #define I_DONT_LIKE_THE_C_LANGUAGE_SO_ILL_MAKE_IT_INTO_BASIC  103 [0] [2010-07-13 15:38:10] Dominique Using register_global on with PHP... 104 [0] [2010-07-13 15:46:43] Kris Problems with Has-A vs. Is-A. I've seen something like this: public class FooFactory { private HashMap<String, Foo> foos; public Foo getFoo(String name) { return foos.get(name); } }  Turned into this: public class FooFactory extends HashMap<String, Foo> { public Foo getFoo(String name) { return this.get(name); } }  When the factory did other things than function as a Map. The above may occur when someone moves to a OO language for the first time. 105 [0] [2009-11-05 18:42:24] Wulfbane I found this in some code a while back: int four = Convert.ToInt32("4");  106 [0] [2009-11-05 18:52:22] PaoloVictor I've actually seen some people using Bubblesort (implemented by themselves, obviously) because they didn't know about Quicksort/Mergesort or thought that their program would need to do "complex comparisons" and that "qsort only sorts ints, floats and doubles". Bubblesort can be faster in some circumstances than quick/merge sort. Use standard library classes and functions (std::vector, std::sort), make the code clean and simple and only optimise if you can prove that the function is performing slower than it could. - reece 107 [0] [2009-11-05 19:06:58] BlairHippo Doesn't understand how to use comments. May take one of two extremes. There's your blindingly obvious waste-of-keystrokes commenter: cakes++; // Increment the counter keeping track of the number of cakes ... and there's the "Comments are ALWAYS a complete waste of time!" religious fanatic. If you truly think your code is opaque unless you describe every tiny detail of what it's doing, or if you've never once encountered a comment that told you something you DESPERATELY needed to know and otherwise would have learned The Hard Way ... yeah. Either way, I call green. (1) Comments should document why something is done a particular way if it is non-obvious -- e.g. handling quirky behaviour of a Windows API call. - reece 108 [0] [2010-07-13 12:40:07] Salil a=4 if (a==5); alert("a is 5") // this will always execute as we use ';' after if condition  AND if (5==a) // this is same as a==5 but sounds like "blue is sky"  Lols at 5==a never thought of that. =D - Cipi The first example seems like less of a mistake from inexperience and more just a typo that even an experienced programmer could make. The second has a very valid justification: putting the constant on the left as a habit avoids the other common accident of using an assignment operator(=) instead of a comparison operator(==) since it will throw a compile error. - JohnFx Using an assign instead of a compare is a rookie mistake, so thus fits the bill nicely. If you're still doing that after 10 years, there are other issues at play. - David Lively 109 [0] [2009-10-17 03:50:34] Peeter Joot Returning pointers to stack variables is one that I've seen green programmers do a number of times. int * blah() { int x ; return &x; ; }  char * foo() { char x[3] ; return x ; }  (with implied use in the caller). In all fairness, lots of non-green programmers do the same thing, but we find less obvious and harder to debug ways to do it (like saving the address to a structure somewhere, and losing track of where we were when this was done). 110 [0] [2009-10-17 04:17:17] O. Askari Using a method of a class inside that class and the method happens to be something that needs to be static. public class MyClass { public int GetRandomNumber() { ... } public void MyMethod() { MyClass c = new MyClass(); int number = c.GetRandomNumber(); // Do the rest of the job without using c } }  111 [0] [2009-07-13 16:15:33] bernie Uninitialized pointers (with a check for NULL -- because the application may have crashed at some point when trying to dereference NULL): char *ptr; if (ptr != NULL) { ... }  112 [0] [2011-01-17 21:36:20] Chris Lively Another sign of an inexperienced programmer: One who insists on doing things their way, even after being shown that best practices dictate otherwise along with full details on why those best practices exist. Worse, one who will not provide any reasoning why their way is better than said best practices other than to say "I disagree." Worse still, one who insists that some certificate (like MS Gold Partner status) gives them license to ignore the best practices and continue doing things in a wrong manner without providing any explanation at all. And, worst of all: when the way they do things results in huge security holes for the client including exposing all source code (and hence intellectual property) to the world. 113 [0] [2011-08-17 17:21:54] nickyt • They tell you "... works on my machine ..." • They never get latest from source control... or rarely • Afraid to debug. They seize up and just say something crashed without debugging it. • They don't look at logs • In Visual Studio.NET I see it time and time again. People just press F5 to build and debug a web app. Just CTRL + Shift B it and reattach the debugger to IIS or the built-in web server and just refresh the page you have open in the browser once the debugger is attached. And they still do it after you tell them the quicker way. Maybe these aren't inexperienced points, maybe they're just points about bad programmers ;) 114 [0] [2011-09-12 06:36:03] Majid Fouladpour I saw this in a question on SO [1] today. It's jQuery: var$id = $(this).attr('id');  @redsquare was first to notice it and commented:$(this).attr('id') - this.id please

[1] http://stackoverflow.com/q/7383933/66580

115
[0] [2011-09-18 19:36:23] stinky472

I've read through all of these answers and I think there's no way to avoid subjectivity here.

Frankly, I've worked with many developers of varying skill levels and experience and I've found that experience doesn't always yield a good programmer. Sometimes it can create a dinosaur of a programmer who can't learn or refuses to learn the most basic things (like how to use a profiler).

The epitome of a bad experienced programmer in my opinion is the one who just can't seem to learn anything new: the close-minded and stubborn type. I even met a developer who had been coding for decades without even knowing what coupling and cohesion meant or even the basic concepts of why they are important to a system.

The epitome of a bad inexperienced programmer is one who is a bit too eager to apply new things he has learned: the overzealous type who sometimes goes overboard with a concept and loses sight of practicality.

One's not necessarily any better than the other. Really a good programmer should constantly analyze his work and look for ways to improve. Both of the bad types of programmers I described above have the problem of being too settled in their ways whether it's due to inexperience and wanting to apply something learned or being experienced but not wanting to try anything new.

116
[-1] [2009-07-22 17:14:42] dar7yl

It's when you come up to your new hire and find him reading "C for Dummies".

I was really looking more for what mistakes a novice programmer makes in CODE. Even if you expand the question, I hardly think it is a mistake for a novice programmer to be reading a beginner book on programming I'd hardly call that a mistake. - JohnFx
In this case, the hire professed to be a competent programmer, and I didn't challenge him on it. He hit his limit after about 2 weeks, and that's when the dummy book was brought up. My fault for not vetting his resume, and allowing other people to override my doubts about this candidate, and passing over a much more qualified person. - dar7yl
117
[-2] [2009-06-24 21:00:45] dwelch

Passing structures across compile domains. Passing structures in general.

not understanding the dangers of malloc(), strcpy(), strlen(), scanf(), etc.

Trying to use an equal with floating point numbers. (In general not understanding floating point while trying to use it).

Using ghee whiz features of a language or tool, just because it makes them feel special or superior, etc. Or just because. Not understanding the cost or risk.

Using a new (to them) language on a project just because they wanted to learn the new language.

Never learning assembly. Never disassembling and examining their code.

Never questioning things like globals, a single return per function, goto's. That doesnt mean use them that means question the teacher (after you pass the classes and get your diploma).

Not understanding the dangers of local variables. Not understanding the dangers of local globals (locals with the word static in front of them).

Doing things like this: unsigned int *something; unsigned char *cptr; ... cptr=(unsigned char)something; ... Then using *cptr or cptr[]

Doing things like this: unsigned int IamLazy; IamLazy=I.am.too.lazy.to.type; Just because you are to lazy to type. Not understanding the implications of that action.

If you cannot install the software you wrote on a computer, you are not a developer. If you cannot install the operating system on a computer then install your software you are not a developer. If you cannot repair the operating system on the computer that runs your software, you are not a developer. If you cannot build a computer from a box of parts (motherboard, memory, processor, hard disks, etc) well I will let it go, normally that is the first task on the first day of your first job, then the os, then the compilers/tools. If you make it that far then you might be allowed to write code.

We have people who do that (installing tools, etc) whose time is a lot less expensive than mine. I can do it, but it's not really part of my job description. Besides, many companies require drive encryption, software push, administrative scan and other gadgets to be installed on dev machines, which requires access or domain rights that really shouldn't be handed out to non-admins. - David Lively
118
[-3] [2008-10-26 00:19:47] Dustin Getz
[1] http://stackoverflow.com/questions/114342/what-are-code-smells-what-is-the-best-way-to-correct-them

119
[-13] [2008-10-26 00:21:13] ironfroggy