1. 3
  1.  

  2. 2

    I agree with most of these refactorings, but I think extracting the function reduceWeightSum hurts more than it helps.

    function reduceWeightSum(sum, item) {  
      return sum + getWeightByType(item);
    }
    function getCollectionWeight(collection) {  
      return getCollectionValues(collection).reduce(reduceWeightSum, 0);
    }
    

    reduceWeightSum is too tightly coupled with its site of use to reuse it in any other situation. And its name isn’t clear enough to make me confident that the function is correct when it is called with .reduce(reduceWeightSum, 0).

    It’s true, inlining the helper function makes getCollectionWeight kind of messy:

    function getCollectionWeight(collection) {  
      return getCollectionValues(collection).reduce(function(sum, item) {
        return sum + getWeightByType(item)
      }, 0);
    }
    

    But you can refactor it in a better way by chaining a map and then a reduce:

    function sum(array) {
      array.reduce(function(sum, val) { return sum+val; }, 0);
    }
    function getCollectionWeight(collection) {  
      return sum(getCollectionValues(collection).map(getWeightByType));
    }
    

    The calculation would probably be slightly slower this way, but if that mattered, you could use a library such as Lodash to make the map lazy. Lodash also already contains a _.sum function.

    1. 1

      Whenever I see this kind of advice I miss the counterbalancing nuance: make sure you don’t let the little functions you extract become part of your published interface. You really don’t want your colleague importing one of your helper functions in their project!