In this linting series, we've now stumbled across the fabled function override bug.
this.getReportData = function(type, reportname, id) { ... };
// 100 lines later
this.getReportData = function(name, id, year, month) { ... };
Now, what happens is that only the last function is defined as getReportData because there is no such thing as method overload in javascript, like in C#. So... my task was set to linting, not refactoring an entire component. So what I had to do, was just get past this, because doing smoke testing, I just can't ignore this and keep on trucking as if I hadn't seen it.
"This isn't right"
So just to get on with it, I had to implement something that doesn't feel right. What it does is it turns a function into something that can return different types of objects. Now ESLint won't catch this, because it cannot analyze the full code flow, so it won't know this. It didn't even catch the function override, because it wasn't a named function, but a variable being overriden which is allowed in JS. And some dynamic scripts being loaded by JSONP rely on being able to override functions in a parent scope or otherwise.
Now... to make a function overload, I had to make a new function, that took any number of parameters. Which btw, all functions in javascript do, just that you get named parameters if you define them in the function parameter declaration.
"Overload! Overload!"
Now below "solution"/code could have been written as function(arg1, arg2, arg3, arg4) but... I just wanted to simplify it for myself and just do function().
The second part of this, is of course to create something we call a technical-debt task. So that it's not forgotten and can be fixed later if not fixed through organic growth later on. The code/component should be refactored to use the two NEW function definitions instead of the single getReportData.
So to the "solution"....
this.getReportData = function() {
var firstParam = arguments[0];
var typeParams = ["dashboard","list","details"];
if(typeParams.indexOf(firstParam.toLowerCase()) > -1) {
self.getReportDataByType(arguments[0], arguments[1], arguments[2]);
} else {
self.getReportDataByNameId(arguments[0], arguments[1], arguments[2], arguments[3]);
}
};
this.getReportDataByType = function(type, reportname, id) { ... };
this.getReportDataByNameId = function(name, id, year, month) { ... };
So now we have getReportData function simulating "method overloading" and I didn't have to dig into the dependent components and refactor it and try to ensure that it's all still working and not missed some reference somewhere.
Now I just need to load up JIRA and create the TD task. Wish me luck.
Top comments (0)