refactor(Operator): replace operator<T, R> to interface

[kwonoj]

Description:

This PR replaces implementation Operator<T, R> into interface by base implementation Operator::call is not necessarily needed, since creating new operator requires to override it any time. Instead, this PR makes operator as contract interface only.

[tetsuharuohzeki]

I agree this change

[trxcllnt]

@kwonoj initially I wrote the Operator as a base class because I intended to have a singleton instance on the Observable class's prototype, which would allow us to remove the branching in subscribe. I'd have to circle back around to see if this would hook up the subscription chain in the right order (might require a change in the Subscriber constructor), but if so, it'd look something like this:

export class Operator<T, R> {
  call(subscriber: Subscriber<R>, source: any): TeardownLogic {
    return source._subscribe(subscriber);
  }
}

export class Observable<T> implements Subscribable<T> {
  /* ... */
  subscribe(observerOrNext?: PartialObserver<T> | ((value: T) => void),
            error?: (error: any) => void,
            complete?: () => void): Subscription {

    const sink = toSubscriber(observerOrNext, error, complete);

    sink.add(this.operator.call(sink, this));

    if (sink.syncErrorThrowable) {
      sink.syncErrorThrowable = false;
      if (sink.syncErrorThrown) {
        throw sink.syncErrorValue;
      }
    }

    return sink;
  }
}
Observable.prototype.operator = new Operator<any, any>();

[kwonoj]

Maybe I'm not catching all correctly, is it need all implementation of operator have base classes instead of Observable have specific impl for those behaviors like below?

export interface Operator {
  call(...): TearDownLogic
}

class DefaultOperator<any, any> implements Operator {
  call(..): { return source._subscribe(...) };
}

export class Observable {
  private operator = new DefaultOperator();
}

[trxcllnt]

@kwonoj yes, or even a private operator function on Observable:

export class Observable {
  private operator(source): TeardownLogic {
    // `this` refers to the Subscriber, because
    // Observable does `this.operator.call(sink, this)`
    return source._subscribe(this); 
  }
}

[kwonoj]

Rebased. I think this change can be checked in and @trxcllnt 's change can follow later. Suggestions? /cc @trxcllnt , @jayphelps

[coveralls]

Coverage increased (+0.01%) to 96.722% when pulling ae0c163 on kwonoj:interface-operator into 8f0dc01 on ReactiveX:master.

[coveralls]

Coverage increased (+0.01%) to 96.722% when pulling ae0c163 on kwonoj:interface-operator into 8f0dc01 on ReactiveX:master.

[coveralls]

Coverage increased (+0.01%) to 96.722% when pulling ae0c163 on kwonoj:interface-operator into 8f0dc01 on ReactiveX:master.

[jayphelps]

LGTM

[kwonoj]

cool, thanks @jayphelps ! :)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.