We had an Indian Developer who wasn’t very good and he wasn’t that great at communicating in English. I don’t mind people starting out as a Junior and improving over time, but sometimes I think you need to have a mindset for quality and care about your work, and Manikandan didn’t seem to have that.
Text
Making a change which involves updating text is one of the easiest changes in software development. This particular area of the code had unit tests to check the message we were displaying to the user. So if the requirement changed, you would have to update the unit tests, then change the actual logic. Manikandan changes the code, but for some reason puts an extra space at the end, and doesn’t update the unit tests.
“As suggested the changes were made and PR got patched.”
Manikandan
Another change he did involved changing the file path of where a specific file (sqlite) was meant to be. He ends up changing the file path for some other file instead. I pointed it out and he claimed he had tested it.
“I have dev tested this yesterday, it looks like i have missed sqlite folder path.
Mistakenly i have taken up the cache folder.
Will update and patch the PR. Apologies as i have misplaced the path location mistakenly and have corrected it now and done dev testing”
Manikandan
Corrupt Files
When viewing files that have been attached by users, some of these we natively support like text and images, whereas specialist files we allow opening externally. Some we blocked completely.
A new change would allow files to be saved regardless if they were unsupported or we suspected them to be corrupt. We will inform the user of this though, so this was the 2 requirements:
When you attach:
This file “xxxxx.tif” has been successfully attached. However the file format or file extension is not valid. Verify that the file has not been corrupted and that the file extension matches the format of the file.
When you select “open file”:
This file “xxx.tif” cannot be opened because the file format or file extension is not valid. Verify that the file has not been corrupted and that the file extension matches the format of the file.”
He writes the following note in the bug report:
Made a proper message information need to be displayed to the user as well as restricted further attachment of the corrupted file into user record history.
Manikandan
So that sounds like he is blocking suspected corrupted files, which is not the requirement. When I looked at his code changes, it seemed like he had done the wrong thing.
So I ask him:
“Have the requirements changed since the product owner’s comment? It looks like you have the message that should be displayed, and it sounds like the attachment should still be saved.”
Me
he replies
“Corrupted Document will not be attached to the record. Once again checked in the database to confirm”.
Manikandan
So I reply again:
“Who has decided that it shouldn’t be attached to the record? The Product Owner said it should be attached and there should be a specific message shown to the user.”
Me
“Apologies for that, will check once again with product owner”
Manikandan
…
Got the update and have made the changes as per Document gets attached to the record irrespective of the document was corrupt or not. Will show a message to the user regarding the file corruption but allows the user to attach the doc irrespective of its correctness
Manikandan
So it seems he finally understands the requirement. However, when he submitted his change to review, it will show a message on Loading (but not the one the Product Owner asked for), but will still crash on Save so he hadn’t really changed anything.
Finishing my work
I had made loads of changes for a new set of validation, but didn’t have time to finish it off as I got moved onto a different project. I had to hand over my work to him. He made some changes and submitted it to me for review. One file I had already wrote the code to show the new message, but then he had added another IF statement, and added the exact same message, however it had some extra spaces in it! So redundant code, and it was wrong.
Another requirement was
“This Warning should be available irrespective of Admin Org flag...”. Yet he had an If statement to check for the flag.
else if(IsAdminOrg)
{
There should be no IF statement because it should happen regardless if the flag is on or off.
In another code block, his logic was based on a check :
specialisedCode.Equals(SpecialisedCode.HDS3)
This equals check wouldn’t actually ever equate to “true” because the comparison was against different object types. And even if it did work, won’t this display the same text twice? He already added code to add it to the list, then had some code to add it again if the If statement was true:
if (matchedItems != null)
{
var displayName = matchedItems.GetAttribute("displayName");
var specialisedCode = matchedItems.GetAttribute("code"); ;
if (!allComponentNames.Contains(displayName))
{
allComponentNames.Add(displayName);
if (specialisedCode.Equals(SpecialisedCode.HDS3) || specialisedCode.Equals(SpecialisedCode.HDS4))
{
allComponentNames.Add(displayName);
}
}
}
In another part of the code, he had this code
var hasExceptionalComponents = HasExceptionalComponents(sectionDefinition);
if(!data.IsSystemLibraryItem || (data.IsSystemLibraryItem && hasExceptionalComponents))
So if the first part of the if statement wasn’t true (“!data.IsSystemLibraryItem”), then it would evaluate the second part, but data.IsSystemLibraryItem would always be true (since it is just the inverse, and the opposite of “false” is “true”). So that bit could be excluded; so yet more redundant code from Manikandan. This would mean you could write:
if(!data.IsSystemLibraryItem || hasExceptionalComponents)
but what was he doing if this statement was true or false? Here is more of the code:
if (!data.IsSystemLibraryItem || hasExceptionalComponents)
FormListFromTemplateSectionDefinition(sectionDefinition, allComponentNames);
else
FormListFromTemplateSectionDefinition(sectionDefinition, allComponentNames);
That’s right, he was doing the exact same thing, so that code block can actually be one line:
FormListFromTemplateSectionDefinition(sectionDefinition, allComponentNames);
So the majority of the code he writes is just redundant. How can you be so incompetent?
When I flagged it to Manikandan, he said
or i am i missing it somewhere, sorry. lil bit confused on this, as i am alone working on this project.
Manikandan
This isn’t a misunderstanding of the requirements, it’s just a lack of care and thought.
Stranger Coding Tings 