tags:

views:

20390

answers:

116

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

+254  A: 
if (test) {
  return true;
}
else {
  return false;
}
lucas
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
This is a classic one!
Camilo Díaz
Umm... missing close brace?
Rewrite as:return test;:)
korona
I mostly see this one in the form: return test == true ? true : false;
Firas Assaad
Wow... a classic, super answer!!
Shivasubramanian A
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
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
"When they expect to have more there" means that they're not familiar with the concept of YAGNI.
Andy Lester
Thinking the posted code is clearer than "return test" is another sign of an inexperienced programmer.
Dour High Arch
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
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
Or when they do something like "if (true == test) // avoid possible assignment to test"...
GalacticCowboy
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
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
Conditional breakpoints, anyone?
Michael Myers
Of course, an experienced programmer would write: return test ? true : false; // :)
Danko Durbić
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
@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
It's obvious what's wrong with this code: Opening braces should be on separate line as per JSF AV rule 60.
squelart
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
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
+122  A: 
  • 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!");
}
Dave Markle
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
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
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
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
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
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
@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
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
@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"
ck
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.
Dev er dev
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
+106  A: 

Using lots of global variables

Decker
Global variables have been banished and replaced with singleton classes.
Barry Brown
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
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
Neil, a Singleton is just a fancy name for a global object.
user9876
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
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
@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
A: 

Adding lines of code.

ironfroggy
If you meant that as opposed to "Removing lines of code when fixing a bug", I'm totally with you. If not--pretty innane, I take away my +1
Bill K
sorry, you made me laugh. still voting you down though.
Isaac Waller
haha. this made me laugh
KennyCason
Why is everyone laughing? This is so true it isn't funny.
Mike Graham
+182  A: 

This made me a sad panda

bool x = SomeMethod();

//Sometime later

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

if(whatIsX == "TRUE")
{
    //Do Stuff
}
Eoin Campbell
Niiiice.. me like !
Newtopian
Eew, just looking at that makes my toenails curl.
HS
Off to the Island of Misfit Mascots Commune for you!
TraumaPony
That's not 'inexperienced'... It's just horrible.
Ace
ooooohhh... I have seen many ugly lines of code involving boolean logic, but this is by far the most... ineffective.
Peter Perháč
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 http://thedailywtf.com/Articles/The-Test-of-Truth.aspx
Greg Beech
And you'll love this http://thedailywtf.com/Articles/Outsourced_Property_Value.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
is this real? or just made up joke?
ra170
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
ck
. . . . . . . .
Binary Worrier
That was me being speechless . . .
Binary Worrier
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
KennyCason
+19  A: 
  • Reinventing the wheel badly

    class HashTable {
        Object[] keys, values;
        ...
        Object get(Object key) {
            Object found = "file not found";
            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
vt
"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
+1  A: 
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.

EvilTeach
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
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.
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
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
+61  A: 

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.

Glomek
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
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
@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
@Nouveau: _Real_ programmers use butterflies. (http://xkcd.com/378/)
user9876
Listen XML is like violence. If you can't solve your problem with it - you're not using enough
PSU_Kardi
Bit like learning to use Linq (.NET) and then using it for everything which it seems a lot of people do.
Ian
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
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
+87  A: 

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.

itsmatt
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
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
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
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
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
`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 :-/
KennyCason
A: 

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.

Jedidja
+16  A: 

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.

Ryan
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
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
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
+33  A: 

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...

JPLemme
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
+11  A: 

This is the one I see most frequently by far

Imagine there is a method:

string GetConfigFromFile()
{
    return ReadFile("config.txt");
}

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

string GetConfigFromOtherFile()
{
    return ReadFile("otherconfig.txt");
}

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!

Orion Edwards
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. It's painfully obvious that the first 2 were written by an inexperienced programmer. Best answer I've seen so far.
Evan Plaice
Intermediate programmers have only just learnt about parameters?
Dominic Bou-Samra
+62  A: 

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.

Tim
"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
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
+127  A: 

OK, so I was bored ... an 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 inexperieced developer.

If you have never tried to improve "Eliza", you might be an inexperieced 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://www.amazon.com/Emergent-Design-Evolutionary-Professional-Development/dp/0321509366

http://www.amazon.com/Design-Patterns-Explained-Perspective-Object-Oriented/dp/0321247140/ref=pd_bxgy_b_img_b

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/Grue_(monster)

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://www.albahari.com/threading/

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

Heh: http://images.tabulas.com/822/l/dilbert_actual_code.jpg

How do you nest gotos?
paperhorse
I wrote an OS in under 1K of RAM. :-)
Paul Nathan
@Vlion Sticking it in the ROM is cheating! ;)
Jacob
I see you must be going for the longest post...
Chris Lively
@paperhorse:If (x) goto yelse { if (q) goto z else if (m) goto(l) else if ( p) goto(a) else goto(b)}
Nazadus
+1 for Thread.Sleep() *shudders*
RobS
+1 for inexperienced programmers (and managers) thinking design patterns are the holy grail
AaronS
+1 This was brilliant!
Jas Panesar
Beagle Bros--they're those criminals from the Gladstone comics, right? ;)
outis
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
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
If you call yourself a "guru" at anything, you might be an inexperienced developer :)
Not Sure
+1 Lol at #3. Funny cos its true.
Justicle
+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
#3, most bugs are stupid and obvious ... in hindsight.
Casey
+1 for accumulating programming books that you never read. :-0
Tuzo
experience != age
Surya
I can`t divide hex number in my head:)
chester89
`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 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
+83  A: 

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;
    }
}
Orion Edwards
Bill K
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
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
Nice, the classic For-Switch paradigm! http://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
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
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
case 3: Break(); break;
Alterlife
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
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
"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
+6  A: 
public void DoSomething (bool DontDoSomethingElse)
{
}

// Later

DoSomething (!DoSomethingElse);
FlySwat
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
I don't get it, any specific example please?
sundar
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
+11  A: 
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());
bryanbcook
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
+32  A: 

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.

Sherm Pendley
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
+28  A: 

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.

baash05
+41  A: 

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.

Simon Howard
"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
+17  A: 

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

That irritated me.

Paul Nathan
At least the guy has a working knowledge of recursion.
At least it wasn't hyperfactorial.
TraumaPony
how big is N, 2? if so it doesn't matter :pbut really some problems can only be solved in with O(N!) :(
hhafez
If I recall correctly, N was in the 100K range. @[email protected]
Paul Nathan
+77  A: 

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.

Chris Lively
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
Much more insightful than the examples of coding errors posted in this thread.
Barry Brown
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
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
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
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
"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
@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  A: 

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

public type Property
{
    get { return Property; }
    set { Property = value; }
}
Todd White
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
The people that write could like that, tends to debug the stack overflow exception too...
leppie
BTW: Are non-programming related questions "stack overflow exceptions"?
EricSchaefer
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
+12  A: 

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.)

bk1e
Um....deleting null in C++ can cause a run time error.
Whaledawg
I think that's his point
johnc
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
+55  A: 

I've actually seen this:

            bool b;

            switch(b)
            {
                case true:
                    DoSomething();
                    break;
                case false:
                    UndoSomething();
                    break;
                default:
                    MessageBox.Show("Error");
                    break;
            }
pablito
Ahahahahahahha... oh g_d you're serious
Jacob
A bool switch, and whats worse, no breaks :(
Furis
HAhaha, looks innocent till you realise you dealing with a boolean :)
leppie
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
The third option should throw a FileNotFound exception...
Chris Lively
+1 for the WTF reference, Chris.
JohnFx
I guess he/she learned Cobol first and tries to do an EVALUATE.
Luc M
What's worse, the default is actually reachable!b takes up one byte and was never initialized. (2 != true).
Joshua
This pattern is important. It allows you to handle the situation where a quantum singularity transforms the laws of physics.
ebencooke
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
+1  A: 

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;
}
+4  A: 

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

+21  A: 

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

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

Barry Brown
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
+62  A: 

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';
Charles Bailey
but but... uhh... but... Oh... Uhh..What!? Why would you do that!? I'm so confused. Damn Junior Developers.
Kieveli
That is a really great piece of code for interview!
ya23
uh, doesn't strlen search for a \0?
apphacker
apphacker: I think that is actually the whole point to this tongue-in-cheek answer. :)
Arafangion
whoooooooooshhhhh!
Logicalmind
@apphacker: bingo :)
Nippysaurus
argh! that's horrible! :P
Stefano Borini
Oh good god, you actually saw this?
Tim Post
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.
+25  A: 

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

double MyFunction( int intParam )
{
    return localVar = 100 / intParam;
}
Ather
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
drachenstern
+2  A: 

Gratuitous usage of reflection.

dpurrington
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
A: 
  • 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.

Quibblesome
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
+4  A: 

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.

Terry Donaghe
+116  A: 

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

johnc
Indeed, this is a real sign of inexperience.
mafutrct
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
@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
What's wrong with that? I set my compiler to treat warnings as errors, after all...</sarcasm>
Nathan Clark
In Haskell or CAML, it's true!
Dario
Dario, really?.
Stefan Monov
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
A: 

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.

Thedric Walker
+1  A: 
string myVariable;

...

Foo(myVariable.ToString());
Tor Haugen
A: 

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?
Nazadus
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
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
+11  A: 

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.

Stephane Grenier
-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
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
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
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 codeif(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
i would rather go for string.Equals(stringA, stringB) ... that's the most safe technique!
Andreas Niedermair
I'd rather check for the NPE than write something like this. Its really hurts the readability
O. Askari
Ok, so when playing Russian Roulette, its safer to spin the chamber in the OTHER direction?
Tim Post
+17  A: 

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

MadKeithV
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
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
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
+1  A: 

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.

Alberto Zaccagni
+1  A: 

Using a Verdana font to program in....

leppie
Having never done this, could you explain this one further?
Sukasa
As opposed to something like Courier?
Tom Savage
+20  A: 
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.

Jamie Ide
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
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
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
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
+15  A: 

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

John Topley
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
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
+4  A: 

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());
John Topley
What if it's a library class and toString() doesn't give useful info?
finnw
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
+5  A: 

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.

Barry Brown
wow.... that is absurd
ghills
A: 
if(something)
{

}
else
{
  doSomething();
}
NotDan
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
"trying to help the compiler" should be an entry of its own on this list. Ugh.
JohnFx
+1  A: 
//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;
}
NotDan
yes, that last comment is totally wrong. It should be "otherwise i is greater than or equal to 10"! :)
dotjoe
+1  A: 

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);
}
Michael Luton
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 fooconditonal = 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
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
+3  A: 

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?).

48klocs
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
+5  A: 

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 :)

+1  A: 

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)
egaga
+4  A: 

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.
Jeff Thompson
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
+2  A: 

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;
}
Alexander Kahoun
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
+58  A: 

Not being happier to delete code than to write it.

Nothing better than replacing a 30 line monstrosity with one or two liner.
Bernard
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
Best answer so far.
Tim Post
I want to + this 100 times
Chad
A: 

I just saw this

move(0+shift);
Decio Lira
+3  A: 

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 asking questions about assignments.
  • Not documenting specifications and deliverables for their projects.
  • Not reading documentation before asking questions.
  • Not Googling before asking questions.
  • Not spending time familiarizing themselves with their daily toolset.
  • Throwing their two cents into every group discussion.
Jeremy Murray
+2  A: 
Kevin
+3  A: 

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
Dana Holt
+1  A: 

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

BZ
A: 

I've seen the following (or similar) code written both by my current colleague and our predecessor.

some_string[strlen(some_string)] = 0;
ziggystar
+4  A: 

Coding by superstition.

+3  A: 

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. :-)

Paulo Guedes
A: 

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...

Nils-Petter Nilsen
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
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
A: 

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
@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
(contd)statements such as if(MY_CONSTANT < (x*(expr)/y*expr1)) rather thanif( (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.
+8  A: 

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()
jfar
A: 

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.

+27  A: 

Not asking questions when they don't know.

They do ask questions, just not to you :) Typically, its Google.
Tim Post
@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
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
+4  A: 

Re-implementing library functions without realizing it.

David Plumpton
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
+5  A: 

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.)

James
+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
Probably had to do with memory overwriting or alignment.
korona
+4  A: 

int i; // define i as an integer

Alterlife
int i; //define i as integer //comment the fact that i is being defined as an integer
JohnFx
Hey, at least it's not in Hungarian notation.
Barry Brown
+4  A: 

things that boil down to:

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

instead of

some_bool = !some_bool

and for-case structures.

dysmsyd
+1  A: 

Inexperienced software designers often attempt to use the Observer Pattern in multi-threaded software without carefully considering deadlocks and race conditions.

+8  A: 

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.

too much php
A: 

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.

Patrick Gryciuk
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
I do this with PHP because... well, it's already PHP
Carson Myers
A: 

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.
Jin Kim
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
ChrisF
+1  A: 

Multiple nested regions. (.Net)

Jim Evans
A: 

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.

dwelch
+1  A: 

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)?

João Marcus
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
+2  A: 

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.

jprete
+3  A: 

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

rein
+2  A: 

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)
{....}
JohnFx
A: 

Conversions from wide character type to ascii when unnecessary

bobobobo
+2  A: 

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.

LKM
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
A: 
  • 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(...)
    
mandrake
A: 

irrational wishes (of this sort) without regard for readability, maintainability, etc.

SilentGhost
+1  A: 

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!

Slace
Alerts used to be about the only way to do it. Maybe that's a sign of a "too experienced" dev? ;)
Chris Lively
+4  A: 
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.

TM
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
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
A: 

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.

CoderTao
+3  A: 
  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.

hythlodayr
A: 

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.

Kelly French
A: 

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.

Martin Wickman
+1  A: 
  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
DroidIn.net
+7  A: 

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 {}
Dinah
A: 

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)
{
   ...
}
bernie
A: 

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.....";

Kthevar
A: 

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 ".

Jeffrey Kemp
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
A: 

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

dar7yl
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
+1  A: 
@Override
public int hashCode() {
   return 0;
}
df
+3  A: 

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.

John Isaacks
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
A: 
Peeter Joot
A: 

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
    }
}
O. Askari
+1  A: 

I was always fond of

if (x = 1) { ... }

But maybe that is more inexperienced than you were thinking.

Scott S.
A: 

I found this in some code a while back:

int four = Convert.ToInt32("4");
Wulfbane
A: 

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".

PaoloVictor
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
A: 

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.

BlairHippo
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
A: 
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"
Salil
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
A: 

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);
Cipi
A: 

Thinking that Duff's device 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);
    }
}
Niels Basjes
A: 

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
reece
A: 

Using register_global on with PHP...

Dominique
A: 

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.

Kris
+3  A: 

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

Ed B
+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
+2  A: 

calling GC.Collect() regularly to "Clean Up"

Jason w
+1  A: 

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]);
  }
}
TheJuice
+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