21

I am using Retrofit and in every task I have to do something like this:

public class MyTask extends AsyncTask<String, String, String> {

    private void someMethod() {
        final RestAdapter restAdapter = new RestAdapter.Builder()
            .setServer("http://10.0.2.2:8080")
            .build();
        final MyTaskService apiManager = restAdapter.create(MyTaskService.class);
    }

    // ...

}

What is a good way to make this code DRY?

JJD
  • 50,076
  • 60
  • 203
  • 339
birdy
  • 9,286
  • 24
  • 107
  • 171

3 Answers3

46

Both the RestAdapter and the generated instance of your services (MyTaskService in this case) are extremely expensive objects and should be used as singletons.

This means that you should only ever call restAdapter.create once and re-use the same instance of MyTaskService every time you need to interact with.

I cannot stress this enough.

You can use the regular singleton pattern in order to ensure that there only is ever a single instance of these objects that you use everywhere. A dependency injection framework would also be something that could be used to manage these instances but would be a bit overkill if you are not already utilizing it.

Jake Wharton
  • 75,598
  • 23
  • 223
  • 230
  • 1
    Thanks for your response. I benchmarked the create call which uses the Proxy class in retrofit to create the request instance using 'newProxyInstance'. I did this on a device (Nexus 5 running 4.4) for an interface that defined anywhere from 1 to 4 service methods of varying complexity. It took 4-5 milliseconds the first time and 0.3 to 0.5 milliseconds every single subsequent time in the same run. Perhaps I don't understand this but is there a reason you think this is expensive? Also is Android caching the creation after the first time? Just trying to get some clarity, thats all – Rickster Jan 28 '14 at 15:25
  • 1
    It's not expensive in terms of how fast it will run, but rather in terms of memory. Each RestAdapter takes up a considerable amount of memory which is why a singleton would be desirable here, because then you only have one copy in memory at a time. Image you make 10's let alone 100's of requests, each with it's own RestAdapter. The garbage collector can't clean them up fast enough, and you can very easily get out of memory doing this. Android is not caching this, but you can do it very easily with [Singletons](http://www.javaworld.com/article/2073352/core-java/simply-singleton.html). – yiati May 02 '14 at 19:22
  • @JakeWharton Is this still the case? I just created 100 of RestAdapters without significant memory increase. – Eugene Jun 17 '15 at 19:37
  • 3
    @Eugene Yes it's still expensive. You may not notice 100 of it. But if you compare these objects to some everyday throw-away object you will find that it's more expensive by a significant factor of like hundreds to millions. The class was also designed to be reused. It's thread-safe, does lazy loading & caching and a whole lot of work went into optimizing the re-use case. It's like buying new socks everyday instead of washing them. You can get away with it but it just doesn't make sense. – zapl Jul 30 '15 at 18:12
  • @jakewharton there are some place which i use converter in some place which i not..so i have to call explicilty call null where i am not using? – Asthme Nov 03 '15 at 07:47
  • 12
    @jake-wharton Is this still valid for Retrofit 2? – jbxbergdev Feb 06 '16 at 11:35
45

As Jake said, you should use the singleton pattern in order to ensure the same instance is always used.

Here's an example:

public class ApiManager {

    public interface GitHubService {

        @GET("/users/{user}/repos")
        List<Repo> listRepos(@Path("user") String user);

    }

    private static final String API_URL = "https://api.github.com";

    private static final RestAdapter REST_ADAPTER = new RestAdapter.Builder()
        .setEndpoint(API_URL)
        .setLogLevel(LogLevel.FULL)
        .build();

    private static final GitHubService GIT_HUB_SERVICE = REST_ADAPTER.create(GitHubService.class);

    public static GitHubService getService() {
        return GIT_HUB_SERVICE;
    }
}

You can use the service in this example like so:

ApiManager.getService().listRepos(...);
Gautam
  • 4,006
  • 3
  • 32
  • 37
5

first you declare your parent class with all common behavior

public abstract class MyAbstractTask extends AsyncTask<String, String, String> {

 protected void someMethod() { //note that i change private to protected
  final RestAdapter restAdapter = new RestAdapter.Builder().setServer("http://10.0.2.2:8080").build();
  final MyTaskService apiManager = restAdapter.create(MyTaskService.class);
 }

}

then, you extend it with every task

public   class MyTask extends MyAbstractTask {

 //your someMethod() is available from everywhere in your class

}

public  class MyOtherTask extends MyAbstractTask {

 //your someMethod() is available from everywhere in your class

}

but i dont know where you are using restAdapter and apiManager, and if the actually need to be created once per task, since probably you can create it outside of these tasks.

If you create them outside, and then you need to use something inside your task, it is also good to have in mind the Dependency_injection pattern.


Also, you should avoid hardcoding values in your classes, like http://10.0.2.2:8080

You should use at least a final static final String server= "http://10.0.2.2:8080" and then use that, or better, use a setter or the constructor in the most inner class, and set tha values from the activity or the main controller.

Carlos Robles
  • 10,828
  • 3
  • 41
  • 60