3
\$\begingroup\$

I have made the following system to automatically retry network calls if some exception is thrown. (Earlier posted on stackOverFlow, solution inspired by what @Boris answered)

public static <T> Optional<T> autoRetry(@NonNull final DoWork<T> task, @NonNull final Optional<Predicate<T>> resultRejector) {

        for (int retry = 0; retry <= StaticData.NETWORK_RETRY; ++retry) {

            try {

                Thread.sleep(retry * StaticData.NETWORK_CALL_WAIT);
                final T resultAfterWork = task.doWork();
                /**
                 * If the result was not
                 * desirable we RETRY.
                  */
                if(resultRejector.isPresent() && resultRejector.get().apply(resultAfterWork))
                    continue;
                /**
                 * Else we return
                 */
                return Optional.fromNullable(resultAfterWork);
            } catch (InterruptedException | UnknownHostException e) {
                e.printStackTrace(); //To be replaced by proper logging
                return Optional.absent();
            } catch (IOException e) {
                e.printStackTrace(); //To be replaced by proper logging
            }
        }
        return Optional.absent();
    }
  • I take care to not retry after InterruptedException & UnknownHostException.
  • I retry 5 times. After each failure I perform an exponential back off, starting from 300ms going upto 1500ms.
  • The predicate is used to verify the validity of the expected result.

The DoWork class :

public abstract class DoWork<T> {
    protected abstract T doWork() throws IOException;
}

I did not use Supplier because I need to throw an IOException.

Here is an example of how I use it. (this example updates the GCM Id)

//Context is android specific stuff, basically if app dies, reference will return null.

public static boolean updateGCM(final long id, final WeakReference<Context> reference) {

final String regId = autoRetry(new DoWork<String>() {
    @Override
    protected String doWork() throws IOException {

        final Context context = reference.get();
        if(context == null)
            return "QUIT";
        return GoogleCloudMessaging.getInstance(context)
                .register("XXXXXXXXXX");
    }
}, Optional.<Predicate<String>>of(new Predicate<String>() {
    @Override
    public boolean apply(@Nullable String input) {
        return TextUtils.isEmpty(input); //I need to retry if the returned GCM ID was null/empty
    }
})).orNull();
//Context becomes null when the application is exited
//If retires failed or application exited, we return
if(TextUtils.isEmpty(regId) || regId.equals("QUIT"))
    return false;

Log.i("Ayush", "Uploading newGcmId to server");
final Boolean result = autoRetry(new DoWork<Boolean>() {
    @Override
    protected Boolean doWork() throws IOException {

        //if everything is fine, send id to server
        return true;
    }
}, Optional.<Predicate<Boolean>>absent()).orNull(); //No need of predicate hence send absent

return !(result == null || !result);
}
  • So on a scale of 1 to 10 how professional is this approach ? :)
  • Also I do not want to retry if the network is genuinely failing, like UnknownHostException, or something bad happens which wont get resolved with retrying. Any possibility of improving exception handling ?
\$\endgroup\$
1
\$\begingroup\$
public static <T> Optional<T> autoRetry(@NonNull final DoWork<T> task, @NonNull final Optional<Predicate<T>> predicate) {

No predicate, please. Call it something like acceptResult or resultRejector... you know what I mean.

Thread.sleep(retry * StaticData.NETWORK_CALL_WAIT);

This doesn't do what you write, as the first time retry=0.

I take care to not retry after InterruptedException & UnknownHostException.

The latter may be retryable as it may be a temporary network or DNS problem. But it's your choice.

final String regId = autoRetry(new DoWork<String>() {...

I'm too lazy to copy it in my Eclipse and without it, it's unreadable as I have no idea where the expressions ends. What about a private final DoWork<String> regIdDoWork = ... or alike?

Optional.<Predicate<Boolean>>absent()).orNull(); //No need of predicate hence send absent

How nicer would the world be if there was no Optional? :D Just null.

Note that even if you like Optional, you should probably use it for as return types only. This is the place where it may prevent bugs and where it doesn't add so much verbosity.

\$\endgroup\$
  • \$\begingroup\$ The first retry 0 is on purpose. As for your last point, I never had a problem with null, but the more exp people say null is ugly xD \$\endgroup\$ – Dexter Apr 23 '15 at 9:26
  • \$\begingroup\$ @Dexter Yes, null is ugly, but not as ugly as Optional. YMMV, but I fully agree with this rant. ;) \$\endgroup\$ – maaartinus Apr 23 '15 at 11:47
  • 1
    \$\begingroup\$ That's one hell of a rant. I like the idea of every type having a default null replacement value. But it does seem an even more herculean task. \$\endgroup\$ – Dexter Apr 23 '15 at 11:56

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.