1
\$\begingroup\$

This seems to work fine however I'm sure there must be a neater way. The outcome should be: If isHidden == true then call canHideAccount. canHideAccount returns a Single. If isHidden != true then there is no need to call canHideAccount as it is a long asynchronous operation. If canHideAccount returns false then the user should be shown an error dialog via getView().render(). If canHideAccount is true or isHidden was false then updateExternalAccount should be called. This is a network request that returns a Single. If the request completes successfully then update the UI, if there is an error then notify the user.

Kicking off the chain with the Single.just(isHidden) seems a bit odd, but I cant see how to kick it off with the condition that it should only call canHideAccount if isHidden == true.

All comments and criticism are welcome.

public void updateAccount(String accountId, boolean isHidden, String type) {

    UpdatePostData accountUpdatePost = new UpdatePostData();
    accountUpdatePost.accountId = accountId;
    accountUpdatePost.isHidden = isHidden;
    accountUpdatePost.type = type;

    postAccountUpdate = Single.just(isHidden)
            .subscribeOn(Schedulers.io())
            .flatMap(hideAccount -> {
                if (hideAccount) {
                    return canHideAccount(accountId);
                } else {
                    return Single.just(true);
                }
            }).flatMap(canHideAccount -> {
                if (canHideAccount) {
                    return getApi().updateAccount(accountUpdatePost);
                } else {
                    return Single.error(new IllegalStateException());
                }
            })
            .observeOn(AndroidSchedulers.mainThread())
            .subscribe(response -> {
                if (isViewAttached()) {
                    getView().render(State.accountUpdated);
                }
            }, error -> {
                if (isViewAttached()) {
                    if (error instanceof IllegalStateException) {
                        getView().render(State.cannotHideAccount);
                    } else if (isAuthenticationError(error)) {
                        getView().render(State.reauthenticate);
                    } else {
                        getView().render(error);
                    }
                }
                GA.instance.error(error);
            });
}
\$\endgroup\$
  • \$\begingroup\$ 1. starting with Single.just() seems perfectly fine for me - there's no other source of observables in what you've shown 2. is updateAccount() called in an rx chain itself? 3. where is that method/function located? Activity? 4. I'd rather change the whole structure - in your function there are a lot of side effects, which aren't perhaps that much of side effects - updating ui, calls to api... I believe dividing the chain into smaller chunks, and returning some sort of state to be consumed later (preferably in some rx chain) would be better :) \$\endgroup\$ – Antek Aug 13 '18 at 8:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.