Fixed leak when executing UseCase multiple times.

[amatkivskiy]

Hi guys,

Here is the pull request that fixes #135 .

[autoool]

@amatkivskiy hi，I think you can describe the reason about leak bug. why you change code like this? how reproduction this leak? thanks!
I find the code didn't change.

[amatkivskiy]

@chaozaiai Hi, the problem that we are not checking if there was previously subscribed Subscriber.

That means that if you trigger usecase second time you will have leaking first Subscriber because unsubscribe was not called before. This issue can be avoided by calling 'useCase.unsubscribe()' before calling useCase.execute(Subscriber). So I proposed solution to move this code into use case so no action required from the developer.

[Yukooo]

I am sorry but to do unsubscribe is not flexible , maybe you'd better just add some flag like "is Running" and you can check so if it is running then do not execution but if you still want to repeat a request then just call unsubscribe method in a presenter

[amatkivskiy]

@Yukooo Thanks for the point. Can you describe the case when it is not flexible?

[Yukooo]

Here is my version that I use
`/**

• @param The data element type of this use case

• @param The response element type of this use case
  */
  public abstract class UseCase<D, R> {
  private Subscription mSubscription = Subscriptions.unsubscribed();
  protected D mData;

  /**

  • Set a request data.
    *
  • @param data The data to be useful for making a execution.
    */
    public void setData(final D data) {
    mData = data;
    }

  /**

  • Builds an {@link rx.Observable} which will be used when executing the current {@link UseCase}.
    */
    protected abstract Observable buildUseCaseObservable();

  /**

  • Executes the current use case.
    *
  • @param useCaseSubscriber The guy who will be listen to the observable build

  •                      with {@link #buildUseCaseObservable()}.
    

    */
    public void execute(Subscriber useCaseSubscriber) {
    mSubscription = this.buildUseCaseObservable().subscribe(useCaseSubscriber);
    }

  /**

  • Stops this use case.
    */
    public void stop() {
    if (!mSubscription.isUnsubscribed()) {
    mSubscription.unsubscribe();
    }
    }

  /**

  • Defines whether this use case is performed.
    *
  • @return {@code true} if it is performed, {@code false} otherwise
    */
    public boolean isRunning() {
    return !mSubscription.isUnsubscribed();
    }
    }`

And in presenter I can check whether a UseCase is being performed (using isRunning()) so as result presenter can check some condition and decide whether it can stop current task or just give it a chance to finish

[Yukooo]

@amatkivskiy I forgot to say about a configuration changing, so there is case:
Suppose some UseCase takes 10 seconds for execution
and there is next flow:

• onResume
• start executing a UseCase
• a configuration changing
• onResume
• stop previous request and start executing a UseCase AGAIN!
  I mean if an user makes a lot of work of screen orientation then he will never see the response till he stops playing with changing of screen orientation

[amatkivskiy]

@Yukooo Sorry, for late response, I was on vacation. Regarding first comment:
It depends on your approach on who is responsible for handling such situations. I prefer to protect consumer (I mean Presenter) from shooting into his own leg. Especially considering the fact that implementation of this check inside UseCase doesn't take much effort.

Regarding second comment:
First of all you should unsubscribe from UseCase in onDestroy or onPause of you activity.
To handle such situation you need implement caching of the response so next time the user change the orientation the result will be obtained from the cache.

[amatkivskiy]

I think this can be closed after latest refactorings and migration to RxJava 2.