CompositeDisposable should assert input

[benlesh]

I ran into a pretty confusing issue today that boiled down to me sticking an undefined in the CompositeDisposable ctor. It might have been nice to assert the error up front, rather than force the developer to dig deeper into the issue.

[mattpodwysocki]

@blesh you mean something like the following check?

if (arguments.length === 1 && arguments[0] == null) { 
  throw new TypeError('Disposable must be specified'); 
}

Or should I be checking each disposable given in the ctor to ensure not null?

[mattpodwysocki]

@blesh proposed change here:

  var CompositeDisposable = Rx.CompositeDisposable = function () {
    var args;
    if (Array.isArray(arguments[0])) {
      args = arguments[0];
    } else {
      var len = arguments.length, args = new Array(len);
      for(var i = 0; i < len; i++) {
        if (arguments[i] == null) { throw new TypeError('Disposable must be specified'); }
        args[i] = arguments[i];
      }
    }
    this.disposables = args;
    this.isDisposed = false;
    this.length = this.disposables.length;
  };

[mattpodwysocki]

@blesh ugh, I have to go further to evaluate each one in turn especially if an array is passed in. Whee!

[benlesh]

I hadn't looked at it, but it sounds right.

var args = Array.isArray(arguments[0]) ? arguments[0] : [].slice.call(arguments);

args.forEach(function(arg) { 
  if(!isDisposable(arg)) { throw new TypeError('Disposable must be specified'); }
});

where there could be some isDisposable implementation that just asserts the contract:

function isDisposable(o) {
  return o && typeof o.dispose === 'function';
}

[mattpodwysocki]

Done!

[benlesh]

:+1: Awesome. I suspect this will save someone some time in the future.