documentcloud / underscore-contrib

The brass buckles on Underscore's utility belt
MIT License
621 stars 117 forks source link

'result' in while loop is wraped in a _() #194

Closed delonnewman closed 4 years ago

delonnewman commented 9 years ago

'result' in the return statement is expecting a value method

fogus commented 9 years ago

What is the purpose of this patch? What problem is it intended to solve?

delonnewman commented 9 years ago

The following code results in the error, "TypeError: result.value is not a function":

var dateRange = _.partial(_.trampoline, function(d1, d2) {
  if ( afterDay(d1, d2) ) throw("error d1 should be earlier or the same date as d2");
  if ( sameDay (d1, d2) ) return _.done([d1]);
  else {
    if ( beforeDay(d1, d2) && sameDay(nextDay(d1), d2) ) return _.done([d1, d2]);
    else {
      return function(){ return _.flatten([d1, dateRange(nextDay(d1), d2)]) };
    }
  }
});
jgonggrijp commented 4 years ago

Thank you @delonnewman for wanting to help. However, the problem was in your code, not in Contrib.

In order to more easily reproduce the error, I created a version of your code that processes integers instead of dates:

function rRange(start, end) {
    if (start > end) throw new Error('end must not be before start');
    if (start === end) return _.done([start]);
    if (start + 1 === end) return _.done([start, end]);
    return function() { return _.flatten([start, rRange(start + 1, end)]); };
}

_.trampoline(rRange, 1, 1) // [ 1 ]
_.trampoline(rRange, 1, 2) // [ 1, 2 ]
_.trampoline(rRange, 1, 3) // TypeError: result.value is not a function

The problem is in the recursive case. _.trampoline requires that every function returns either a new function or a special value created with _.done. Your first recursion, however, returns an array (the result of _.flatten).

This can be fixed in your code by passing the intermediate result to your recursive function invocation.

function rRange(start, end, result) {
    result || (result = []);
    if (start > end) throw new Error('end must not be before start');
    if (start === end) return _.done([start].concat(result));
    return function() { return rRange(start, end - 1, [end].concat(result)); };
}

_.trampoline(rRange, 1, 1) // [ 1 ]
_.trampoline(rRange, 1, 2) // [ 1, 2 ]
_.trampoline(rRange, 1, 3) // [ 1, 2, 3 ]
_.trampoline(rRange, 1, 4) // [ 1, 2, 3, 4 ]

In this example, I'm building up the range back to front, but you could also do it front to back.

I'm closing this now, but if you have any questions or comments, please feel free to discuss!

delonnewman commented 4 years ago

Got it, that make's sense. Thanks for the reply that was sometime ago 😄 .