if (test) {
  return true;
}
else {
  return false;
}

This made me a sad panda

bool x = SomeMethod();

//Sometime later

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

/// ==== EDIT

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

http://www.amazon.com/Emergent-Design-Evolutionary-Professional-Development/dp/0321509366 ^[1]

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

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) ^[3]

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

• 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!");
}

Using lots of global variables

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.

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.

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;
    }
}

Not being happier to delete code than to write it.

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.

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';

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.

I've actually seen this:

            bool b;

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

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.

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.

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

Not asking questions when they don't know.

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.

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

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

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

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

• 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

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.

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

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

That irritated me.

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.

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

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!

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());

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

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

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

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.

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 {}

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.

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

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.

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

public void DoSomething (bool DontDoSomethingElse)
{
}

// Later

DoSomething (!DoSomethingElse);

int i; // define i as an integer

Re-implementing library functions without realizing it.

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

Coding by superstition.

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.

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());

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

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.

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.

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.

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

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

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

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.

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

Gratuitous usage of reflection.

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

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.

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

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

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 ^[1] structures.

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

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;
}

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);
}

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

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.

In web development not understanding the difference between the client and server is something that I've seen quite a few times from new developers.

I've been asked why this didn't work plenty of times (ie - why my doesn't my alert show):

Page.ClientScript.RegisterStartupScript(this.GetType(), "notification", "alert('Success!');", true);
Response.Redirect("/default.aspx");

(I think that code's right :P).

And using alerts for debugging JavaScript, man that is such a frustrating thing to see, particularly when using Firebug!

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

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

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

@Override
public int hashCode() {
   return 0;
}

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

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

I was always fond of

if (x = 1) { ... }

But maybe that is more inexperienced than you were thinking.

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

Thinking that Duff's device ^[1] is a neat feature of the C programming language they can use to improve production code.

Quote from Wikipedia:

send(to, from, count)
register short *to, *from;
register count;
{
    register n=(count+7)/8;
    switch(count%8){
    case 0: do{ *to = *from++;
    case 7:     *to = *from++;
    case 6:     *to = *from++;
    case 5:     *to = *from++;
    case 4:     *to = *from++;
    case 3:     *to = *from++;
    case 2:     *to = *from++;
    case 1:     *to = *from++;
        }while(--n>0);
    }
}

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

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.

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

Multiple nested regions. (.Net)

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)

//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;
}

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.

Using a Verdana font to program in....

string myVariable;

...

Foo(myVariable.ToString());

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

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

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;
}

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.

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.

• Exposing collections as get; AND set;

• Code in the file that doesn't actually do anything but was included in a bunch of changes they implemented to get it working meaning they think the pointless code "somehow" helps.

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

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.

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

some_string[strlen(some_string)] = 0;

I just saw this

move(0+shift);

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?

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.

if(something)
{

}
else
{
  doSomething();
}

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.

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.

Conversions from wide character type to ascii when unnecessary

irrational wishes (of this sort ^[1]) without regard for readability, maintainability, etc.

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.

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.

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.

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

Using register_global on with PHP...

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.

I found this in some code a while back:

int four = Convert.ToInt32("4");

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

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.

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"

Returning pointers to stack variables is one that I've seen green programmers do a number of times.

int * blah()
{
   int x ;
   return &x; ;
}

char * foo()
{
   char x[3] ;
   return x ;
}

(with implied use in the caller). In all fairness, lots of non-green programmers do the same thing, but we find less obvious and harder to debug ways to do it (like saving the address to a structure somewhere, and losing track of where we were when this was done).

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

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

Another sign of an inexperienced programmer:

One who insists on doing things their way, even after being shown that best practices dictate otherwise along with full details on why those best practices exist.

Worse, one who will not provide any reasoning why their way is better than said best practices other than to say "I disagree."

Worse still, one who insists that some certificate (like MS Gold Partner status) gives them license to ignore the best practices and continue doing things in a wrong manner without providing any explanation at all.

And, worst of all: when the way they do things results in huge security holes for the client including exposing all source code (and hence intellectual property) to the world.

• They tell you "... works on my machine ..."
• They never get latest from source control... or rarely
• Afraid to debug. They seize up and just say something crashed without debugging it.
• They don't look at logs
• In Visual Studio.NET I see it time and time again. People just press F5 to build and debug a web app. Just CTRL + Shift B it and reattach the debugger to IIS or the built-in web server and just refresh the page you have open in the browser once the debugger is attached. And they still do it after you tell them the quicker way.

Maybe these aren't inexperienced points, maybe they're just points about bad programmers ;)

I saw this in a question on SO ^[1] today. It's jQuery:

var $id = $(this).attr('id');

@redsquare was first to notice it and commented:

$(this).attr('id') - this.id please

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

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

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

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

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

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

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.

what-are-code-smells-what-is-the-best-way-to-correct-them ^[1]

Adding lines of code.