Incompetent Developer Tales Part 2

Although Junior developers can be useful, I have been against my employer’s over-reliance on them. If you get someone cheap with high potential, as long as you reward them, you end up with someone that knows your system, loves the company and is a great developer.

The problem is that we love hiring them but not rewarding them which means the best ones leave and the bad ones remain. The focus on cheap wages has led to more and more offshoring which has led to the rapid expansion of our Indian office. How do you hire so many people quickly? lower the standards. So now you have a high amount of incompetent people but they are cheap.

It’s not the fact they are Indian that is the problem, it is the fact the demand is high and the standard we had in hiring is low. The problem this has in the work culture is that it is easy to see a discrepancy in quality between the UK and Indian developers as a whole; which means you end up seeing them as inferior, despite some of them actually being good. The good ones tend to be the ones we hired in Senior positions, so they would naturally have higher wages anyway.

So building on from my recent blog on one particular developer, Here’s a collection of things other people have done:

Rollbacks

James has just rolled back someone’s changes who merged into the wrong folders.Don’t they think something isn’t right when it is showing [add] next to the main Database folder. Looks like they copied the folders up one level so now it is re-adding everything as a duplicate.

Dean 16:27:
haha
Me 16:28:
that change by Portia is mad when you look at the changesets
original patch
change to patch,
fix db patching errors,
rollback,
rollback other changes,
rollback from xml file,
then Chris comes into undo the rollback
Dean 16:30:
it's just wrong that we've got people who don't know what they're doing
Me 16:30:
but it's cheap
Portia went wild on that second “rollback” and manually reverted the files.
removed 8 patches and added 1, instead of removing 1
it's amazing how many rollbacks happen these days
Dean 16:40:
Rollbacks worry me in general

“used for identification”

In general, SQL databases are designed to reduce redundancy. So if you have a table storing a list of “job roles”, then if another table references this information, you can link it together via an ID of the row. What you shouldn’t do is copy the data into another table. This means if the data needs to be updated, then you need to remember to update both, and this will double the storage space too.

I saw that a developer was doing this. It was only one column of text, but why were they copying it over into their new table instead of just referencing it?

Me
Is there a reason why this isn't being taken from JobCategory?
It is never returned in a stored proc call so there is no need for it

Vignesh
JobCategoryName used for identification of JobCategoryID and not used in stored proc. Thanks

Me
Regardless if the system uses the data, or if it is there for our Staff to read in the database; you would just write a query that joins onto the JobCategory table.
what if the JobCategoryName in JobCategory is updated? The names in your new table won't be accurate

Vignesh
JogCategoryID only used in stored proc/code, JobCategoryName is just an identification for JogCategoryID in the table. Thanks.

Me
So it needs to be removed?

Vignesh
JobCategoryName was removed since it is used in code or stored proc. Thanks.

Refresh Link

private void llbl_RefreshList_LinkClicked(Object sender, LinkLabelLinkClickedEventArgs e)
{
IEnumerable<ExecutionScheduleDetail> runningItems = _service.GetAllScheduleSearches(AuthenticatedUser.Organisation.Guid);
int count = (from runningitem in runningItems select runningitem).Count();
if (count>0)
{
LoadRunningSearches();
}
else
{
MessageBox.Show(
"This run has just completed and details can no longer be viewed.",
"Running Searches",
MessageBoxButtons.OK,
MessageBoxIcon.Error);
LoadRunningSearches();
}
 
}

Instead of just getting the count from the Ienumerable, they are selecting all the items they already have. Then since they are calling the LoadRunningSearches method in both parts of the IF, it may as well be moved out. So then the code would just be

if count == 0 , show message box.

when a review comment was left about not specifying the method call twice, he then just moved the GetAllScheduleSearches to a field, which meant it would no longer get the latest searches every time you clicked. Since the link is to “Refresh”, it wasn’t doing what it was supposed to.

If Constraint Exists

 if exists (Select * from sys.check_constraints where name = 'DF_Templates_AgeRange_AgeTo ')
 alter table Templates.AgeRange drop constraint DF_Templates_AgeRange_AgeTo;

I noticed there was a redundant space at the end of the constraint name so I thought it was likely that the check would never be true (unless SQL ignores it automatically?)

Me 4/28/2022
Is this space intentional?
Vignesh 4/28/2022
Removed
Joel 4/28/2022
Do you actually need the select check? I think you should be able to use the dependent patch mechanism instead.
Vignesh 4/28/2022
I think it is not needed, we checked the condition to avoid any error. if its true it will execute
Joel 4/28/2022
The constraint is added in patch 7.809, on which you've marked this patch as dependent. So this patch will literally only run if the constraint was created successfully.

 We have an attribute in the xml so you can state dependent patches, so will only run if the prerequisite patch has run. Vignesh was aware of it because he had used it, but then he also had this guard clause that possibly didn’t even work.

When told he didn’t need it, he then agrees that it isn’t needed, yet, put it in there so it would “avoid error”. Does he mean there was an error? Or just being overly cautious?

Spelling

inActive ? ResourceStatus.Inactive : ResourceStatus.Active

Sometimes, it’s the little things like this that annoy me. How can you write “inActive”, and not realise that you either:

  • have spelt “inactive” wrong,
  • or alternatively – someone else has when they created this enum

Therefore why did they not fix it? There’s clearly an inconsistency there.

In a similar fashion, I saw this recently:

//Recieved and Transalted

Both words are spelt wrong. It was also copy and pasted from another file. It does pose a good question though, if you copy and paste, do you think you should correct the spelling or leave it for maximum laziness? I guess the advantage is if you search for that text to try and find the original code, it’s better to match it as much as possible.

throw new NotSupportedException("Can't able to fetch template consultation details!");

Indians always seem to write “Can able” and “Can’t able” instead of just “can” and “unable”.

Untested code

string.Format("{0} {1}", _isRegistered ? "Not Registered:" : "Registered:", statusCode)

The logic was consistently backwards. It wasn’t a case that they typed it wrong and didn’t bother testing it. There were several files with the same type of logic. I pointed it out and they rewrote the entire thing.

Leave a comment