1

I am making a network call using rxJava2, and based on the response (either success or error), I have to move my work forward on UI thread.

I have written the code below. It seems working fine.

       WApi wApi = ServiceGenerator.createService(WApi.class, sURL);
       dataManager = InventoryModule.getDataManager();
       rx.Observable<GetFeature> getFeatureObservable = 
       dataManager.executeGetFeature(caseId, wrapperApi);
       if (getCV2FeatureObservable != null) {
           try {

              getFeatureObservable.subscribeOn(Schedulers.io())
                       .observeOn(AndroidSchedulers.mainThread())
                       .doOnError(throwable -> {
                           Log.e(TAG, "Err::" + throwable.getMessage());
                           // init default values because of error response
                           initDefaultValues();
                           // No data to use from WS call. Move forward with 
                           //cookie from SSO auth
                           cookieReceived(userID, cookieData, patchNo);
                       })
                       .onErrorResumeNext(rx.Observable.empty())
                       .subscribe(getFeature -> {
                           // use the data from WS response
                           processAndUpdateFeature(getFeature);
                           // move forward with cookie from SSO auth
                           cookieReceived(userID, cookieData, patchNo);
                       });
           } catch (Exception e) {
               Log.e(TAG, e.getLocalizedMessage());
           }
       }

Still I need opinions, Am I doing it right? Am I missing something? or can I use other operators and make it better? the way I am placing my UI work into corresponding operators, will it work properly in both error or success response?

003
  • 197
  • 1
  • 12
  • `.observeOn(AndroidSchedulers.mainThread())` is the important line here. You are receiving the result in the main thread. So Yes, this is actually the correct way to proceed. – MatPag Aug 26 '19 at 15:32
  • If this is Rx2, then why are you using `rx.Observable.empty()` instead of `io.reactivex.Observable.empty()`? Considering that `rx.*` is from RxJava 1.x. – EpicPandaForce Aug 26 '19 at 15:47

2 Answers2

1

The only questionable choice is all the complications you're doing on errors.

instead of using doOnError + onErrorResumeNext I suggest you move your logic to the Subscriber:

getFeatureObservable.subscribeOn(Schedulers.io())
        .observeOn(AndroidSchedulers.mainThread())
        .subscribe(getFeature -> {
            // use the data from WS response
            processAndUpdateFeature(getFeature);
            // move forward with cookie from SSO auth
            cookieReceived(userID, cookieData, patchNo);
        }, { throwable -> {
            Log.e(TAG, "Err::" + throwable.getMessage());
            // init default values because of error response
            initDefaultValues();
            // No data to use from WS call. Move forward with
            //cookie from SSO auth
            cookieReceived(userID, cookieData, patchNo);
        });

Your thread switching (subscribeOn and observeOn) is fine.

EDIT: One more thing: Unless the processAndUpdateFeature or initDefaultValues or cookieReceived can throw an error, the try-catch block seems unnecessary.

Bartek Lipinski
  • 30,698
  • 10
  • 94
  • 132
  • This seems reasonable to me. Question: in any cases I 'cookieRecived()' is gonna execute, so can we replace it into more concise way? like I was looking into doAfterTerminate(), doOnUnsubscribed(). I am using rxAndroid 1.2.1 – 003 Aug 26 '19 at 17:33
  • It really depends on how many items your `Observable` can emit. If it can emit only a single item, yea you can use `doFinally` (it is called `onError` or `onComplete`). If your `Observable` can emit more than one item (and you want to call `cookieReceived` after every single item emitted and not after all of them), you don't have an easy callback for your case. – Bartek Lipinski Aug 26 '19 at 19:41
  • @Ddp also, I added a suggestion about the `try-catch` block. – Bartek Lipinski Aug 27 '19 at 07:36
  • @BartekLipinskin I got your point for try-catch, thanks. Also ```doFinally``` operator shows deprecated (according to the version I am using). So, can we replace it with ```doAfterTerminate()```. Note: I am going to use ```processAndUpdateFeature``` or ```initDefaultValues``` or ```cookieReceived``` only after the network call is done and at the end when I receive either error or success response. – 003 Aug 27 '19 at 13:34
  • @Ddp `doFinally` is deprecated ? What version of RxJava are you using? `doAfterTerminate` seems fine as well, but I haven't used that so I can't be sure. Here's a SO answer that describes differences: https://stackoverflow.com/a/47307813/1993204 – Bartek Lipinski Aug 27 '19 at 15:18
  • Have question, can this Background work make my Activity go in onPause() or onStop()? – 003 Sep 12 '19 at 14:36
1

In my experience I have had best luck using AsyncTask for network operations.

There are many advantages. 1) Use publish progress to show progress bar advancement. 2) You can have a single place to handle errors in a consistent way while also making the 'success' flow do different things. 3) AsyncTask is an Android construct so you have a good chance of it working consistently between versions.

Fracdroid
  • 1,135
  • 10
  • 15
  • AsyncTask is not recommended for long running tasks. such as network call. – 003 Aug 26 '19 at 17:36
  • @Ddp can you sight a source for your claim? A google search shows many articles that support using async task with networking. Also I have worked on a multiple enterprise grade apps that have used this approach for many years with great success. – Fracdroid Aug 26 '19 at 17:47