3

I have been using Dagger/Retrofit for the past few months and have seen a common pattern of implementing an ApiModule class for an api. These ApiModules typically look something like this:

@Provides @Singleton Client provideClient(OkHttpClient client) {
    return new OkClient(client);
  }

  @Provides @Singleton Endpoint provideEndpoint() {
    return "release".equalsIgnoreCase(BuildConfig.BUILD_TYPE)
        ? Endpoints.newFixedEndpoint(PRODUCTION_URL, "Foo Production Url")
        : Endpoints.newFixedEndpoint(STAGING_URL, "Foo Staging Url");
  }

  @Provides @Singleton Converter provideConverter(Gson gson) {
    return new GsonConverter(gson);
  }

  @Provides @Singleton RestAdapter provideRestAdapter(Endpoint endpoint, Client client,
      Converter converter) {
    return new RestAdapter.Builder()
        .setClient(client)
        .setEndpoint(endpoint)
        .setConverter(converter)
        .setLogLevel(BuildConfig.DEBUG
            ? RestAdapter.LogLevel.FULL
            : RestAdapter.LogLevel.NONE)
        .build();
  }

  @Provides @Singleton FooApi provideFooApi(RestAdapter restAdapter) {
    return restAdapter.create(FooApi.class);
  }

But to clean this up why not do this:

@Provides @Singleton Client provideClient(OkHttpClient client) {
        return new OkClient(client);
      }

@Provides @Singleton Converter provideConverter(Gson gson) {
        return new GsonConverter(gson);
      }

@Provides @Singleton FooApi provideFooApi(Client client, Converter converter) {
    return new RestAdapter.Builder()
        .setClient(client)
        .setEndpoint("release".equalsIgnoreCase(BuildConfig.BUILD_TYPE)
            ? Endpoints.newFixedEndpoint(PRODUCTION_URL, "Foo Production Url")
            : Endpoints.newFixedEndpoint(STAGING_URL, "Foo Staging Url"))
        .setConverter(converter)
        .setLogLevel(BuildConfig.DEBUG
            ? RestAdapter.LogLevel.FULL
            : RestAdapter.LogLevel.NONE)
        .build()
        .create(FooApi.class);
  }

Is there any cons to doing it this way or am I violating some Dagger contract? I ask because there have been cases where I need to use multiple APIs within a project...setting it up like the second example above, makes that possible.

IZI_Shadow_IZI
  • 1,921
  • 4
  • 30
  • 59

1 Answers1

6

There's three reasons to do this:

  1. By splitting the two you are creating a separation of concerns. How the RestAdapter is instantiated and put into the graph is completely separate from how the instance our your service interface is put into the graph. Here you happen to have them in the same module but there's no reason they couldn't be in separate modules or even have one in a library from a different component.

  2. Separate providers allow you override one or both in an override module to customize the behavior without have to know about how the other is used or where it comes from.

    For example, if you wanted to enable different behavior when you are running an integration test you could provide a different RestAdapter instance.

    @Provides @Singleton RestAdapter provideTestRestAdapter() {
      return new RestAdapter.Builder()
          .setEndpoint(Endpoints.newFixedEndpoint("http://mycomputer.local/api"))
          .setLogLevel(FULL)
          .build();
    }
    

    Having this in an override module means you don't have to change where the service instances are being created.

  3. Finally, and most simply, maybe you have multiple service interfaces. You shouldn't be creating multiple RestAdapter instances (unless they are for different endpoints).

    @Provides @Singleton AccountService provideAccountService(RestAdapter ra) {
      return ra.create(AccountService.class);
    }
    
    @Provides @Singleton TweetService provideTweetService(RestAdapter ra) {
      return ra.create(TweetService.class);
    }
    
    @Provides @Singleton DirectMessageService provideDirectMessageService(RestAdapter ra) {
      return ra.create(DirectMessageService.class);
    }
    
Jake Wharton
  • 75,598
  • 23
  • 223
  • 230