Dumb Code List

There was one code review I was having a nosey at, and the Lead Developer tore it apart. It had so many mistakes in it, I thought I’d make a note of the best ones.

1. string.IsNullOrEmpty(null)

We are passing “null” into a method and asking “is this null”? Yes it is. 100% guaranteed to be.

2. new FilingContainer(fEvent) ?? new FilingContainer(fEvent, null); 

This code means: “create a new FilingContainer. If this is null, then create a new FilingContainer (but with slightly different parameters”

Lead Developer’s comment: “how does this constructor return null?”

A constructor can only return a new object, or throw an exception. You can never return a “null” value. So the second part of the statement would never execute.

3. There was a massive constructor defined with many parameters of the same type. So like

public  Ticket(
String firstName,
String secondName,
Int userId,
String userName,
Int jobCategoryId,
String jobCategoryName)

But then when he went to use it, he had the parameters in the wrong order:

new Ticket(
firstName,
secondName,
jobCategoryId,
jobCategoryName,
userId,
userName)

Lead Developer’s comment: “You are setting the JobCategoryId to something called UserId! Which is a hard coded value from your development database, and cannot be relied on to be the same in Live! You are setting the JobCategoryName to the User’s Name!”

So not only were the parameters in the wrong order, he should have queried the database for the correct JobCategoryId in his code. But he had queried it manually, then hardcoded the value in his code, so it would only work on his machine.

4. Title + surname.toTitleCase( ) + “ ?” + firstname;

Lead Developer’s comment: this is “MrSmith ?DAVE” Also, is title-case correct? What if someone’s name is McDonald? Should it be Mcdonald?

Pretty much everything is wrong with the output!

5. Lead Developer’s comment: “Sex or gender, use one or the other”.

Some files they had named a variable “sex” and in others “gender”. Obviously, only one name is correct.

6. Age = (DateTime.Today.Year - messageDetails.User.DateOfBirth.Year).ToString());

Lead Developer’s comment: “Born 31/12/2000 Today 1/1/2001… does not make you 1!”

I’m not sure what sort of people think age is a calculation based on year only, but with that lack of attention, you really need to consider if this is the job for you. Programming is about thinking logically.

7. FullName = Title + Surname; 

Lead Developer’s comment: FullName is just “MrSmith” (nospaces) is that correct? 

They reply “yes there will be no space”

Is that really in the requirements? I highly doubt it. Are they that stupid that they think the Lead Developer is asking how their code currently behaves, rather than questioning if the behaviour is actually correct?

8. There was a variable named “refferalDescription”. Which is an incorrect spelling.

Lead Developer’s comment: “referralDescription?” 

They reply “refferalDescription is new property for code which is used to display in the referral document.” 

I was thinking “How can you write that response and not realise that the spelling is different in his comment to what you wrote?”

9. Referral.Code ?? null. 

This code means, If it is null, then use null. A completely redundant line of code.