4

So Clean Code says you should separate each task to a single function (and add these functions a correct name).

I like the idea, but I've faced this problem so far: you receive a parameter which you want to use also on the topmost function, but you need it on the lowest level as well, so you have to pass the parameter to every intermediate function, which is not cool.

Here's my code:

onUpdate = function(currentSeconds){
    updateRoute(currentSeconds);
    updateAlerts(currentSeconds);
};
updateAlerts = function(currentSeconds){
    for(var i = 0; i < alertMarkers.length; ++i)
        oneAlertUpdate(currentSeconds, alertMarkers[i], alertAll[i]);
}
oneAlertUpdate = function(currentSeconds, alertMarker, alert){
        updateAlertMarker(alertMarker, alert[entryIndex]);
        changeMarkerOpacityIfNeeded(alertMarker, currentSeconds);
    }
}
changeMarkerOpacityIfNeeded = function(alertMarker, currentSeconds){
    //set opacity based on currentSeconds
    var opacity = alertMarker.opacityTime < currentSeconds ? 1 : 0.5;
    if(alertMarker.getOpacity() != opacity)
        alertMarker.setOpacity(opacity);
}
  • the parameter here is currentSeconds it has to be passed, but it's only used on the lowest level
  • please note that I don't use currentSeconds directly in the topmost function, but it's crucial to be passed there, I'll write the reason if you say it's relevant
Ferenc Dajka
  • 175
  • 3
  • 1
    Does this answer your question? [Is there a name for the (anti- ) pattern of passing parameters that will only be used several levels deep in the call chain?](https://softwareengineering.stackexchange.com/questions/335005/is-there-a-name-for-the-anti-pattern-of-passing-parameters-that-will-only-be) – tejasvi88 Aug 23 '21 at 14:19

2 Answers2

2

One option could be to create a "class" (or the JavaScript equivalent in this case), which behaves like a functionoid/functor, and stores currentSeconds, then call into that 'class'[sic] from a proxy function to make sure that you're still only calling onUpdate from the calling code:

var Alerter = (function () {
    function Alerter(currentSeconds) {
        this.currentSeconds = currentSeconds;
    }

    Alerter.prototype.onUpdate = function () {
        this.updateRoute();
        this.updateAlerts();
    };

    Alerter.prototype.updateAlerts = function () {
        for (var i = 0; i < alertMarkers.length; ++i)
            this.oneAlertUpdate(alertMarkers[i], alertAll[i]);
    };

    Alerter.prototype.oneAlertUpdate = function (alertMarker, alert) {
        this.updateAlertMarker(alertMarker, alert[entryIndex]);
        this.changeMarkerOpacityIfNeeded(alertMarker);
    };

    Alerter.prototype.changeMarkerOpacityIfNeeded = function (alertMarker) {
        //set opacity based on currentSeconds
        var opacity = alertMarker.opacityTime < this.currentSeconds ? 1 : 0.5;
        if (alertMarker.getOpacity() != opacity)
            alertMarker.setOpacity(opacity);
    };

    return Alerter;
}());


function onUpdate(currentSeconds) {
    new Alerter(currentSeconds).onUpdate();
}

The main disadvantage to this is more additional complexity in the code, which just negates most of the benefit in avoiding repeatedly passing the same variable(s) around in the first place.

However it avoids that bit of duplication, and the calling code should not notice any difference at least:

var currentSeconds = 5;
onUpdate(currentSeconds);

But overall, I'm not sure whether this approach is really any "cleaner"; the simplest solutions are often the best ones.

Ben Cottrell
  • 11,656
  • 4
  • 30
  • 41
  • 2
    This is the natural approach. Grouping functions together which belong together, especially when they share common parameters is a typical "use case" for a class. – Doc Brown Apr 05 '16 at 10:46
  • Well it seems better, but the rest of the code belongs to the current file (variables referencing to markers, routes, etc.), so I have to pass them to my new Alerter class as well. – Ferenc Dajka Apr 05 '16 at 10:57
  • 2
    @FerencDajka: to me, your comment sounds even more for a reason to clean up the structure of your code by introducing a class with a well-defined input and output. – Doc Brown Apr 05 '16 at 14:02
  • I'm fuzzy on how Alerter has access to the original code's `changeMarkerOpacityIfNeeded()`,`alertMarker` etc. – user949300 Nov 03 '16 at 18:18
0

Do functions like oneAlertUpdate and changeMarkerOpacityIfNeeded have to be visible outside updateAlerts? If not, it could be easy to avoid having to "pass" the values down the call chain.

onUpdate = function(currentSeconds){
    updateRoute(currentSeconds);
    updateAlerts(currentSeconds);
};

updateAlerts = function(currentSeconds){
    oneAlertUpdate = function(alertMarker, alert){
            updateAlertMarker(alertMarker, alert[entryIndex]);
            changeMarkerOpacityIfNeeded(alertMarker);
        }
    }
    changeMarkerOpacityIfNeeded = function(alertMarker){
        //set opacity based on currentSeconds
        var opacity = alertMarker.opacityTime < currentSeconds ? 1 : 0.5;
        if(alertMarker.getOpacity() != opacity)
            alertMarker.setOpacity(opacity);
    }

    for(var i = 0; i < alertMarkers.length; ++i)
        oneAlertUpdate(alertMarkers[i], alertAll[i]);
}