Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

I agree with both @mjolka @mjolka and @kyle-falconer @kyle-falconer. In fact the two points are very similar, because dependency injection frameworks usually work by giving you singletons by default. Mjolka's suggestion of adopting Dagger will be good long-term solution and definitely worth the time investment. Kyle's suggestion is more of a direct, simple solution you can implement right now.

If you go with Kyle's approach, I would point out the modern way of implementing singletons in Java using an enum, that's simpler and more robust:

enum SingletonRestClient {
 INSTANCE;
 private final RestClient restClient;
 private SingletonRestClient() {
 restClient = new RestAdapter.Builder().setEndpoint(BASE_URL).build().create(RestClient.class);
 }
 public RestClient getRestClient() {
 return restClient;
 }
}

You could use this in an activity like this:

public class SomeActivity extends Activity {
 private RestClient restClient;
 @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 restClient = SingletonRestClient.INSTANCE.getRestClient();
 }
}

Of course, you will have to do this in all your activities that need the rest client. But this is still better than forcing all your activities to extend a base activity.

Don't use inheritance just to share data. The right reason to use inheritance is when there is an is-a relationship. Can you say that SomeActivity is a BaseActivity? This sounds inevitably wrong, because the very nature of a "base activity" is that there is just one such activity. A good example would be an EmployeeActivity inheriting from a PersonActivity, which could make sense, since an employee really is a person.

We don't have more details about your context. If there is no is-a relationship between the classes involved, then don't use inheritance, and prefer composition instead: let each class that needs a rest client get it from the singleton.

I agree with both @mjolka and @kyle-falconer. In fact the two points are very similar, because dependency injection frameworks usually work by giving you singletons by default. Mjolka's suggestion of adopting Dagger will be good long-term solution and definitely worth the time investment. Kyle's suggestion is more of a direct, simple solution you can implement right now.

If you go with Kyle's approach, I would point out the modern way of implementing singletons in Java using an enum, that's simpler and more robust:

enum SingletonRestClient {
 INSTANCE;
 private final RestClient restClient;
 private SingletonRestClient() {
 restClient = new RestAdapter.Builder().setEndpoint(BASE_URL).build().create(RestClient.class);
 }
 public RestClient getRestClient() {
 return restClient;
 }
}

You could use this in an activity like this:

public class SomeActivity extends Activity {
 private RestClient restClient;
 @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 restClient = SingletonRestClient.INSTANCE.getRestClient();
 }
}

Of course, you will have to do this in all your activities that need the rest client. But this is still better than forcing all your activities to extend a base activity.

Don't use inheritance just to share data. The right reason to use inheritance is when there is an is-a relationship. Can you say that SomeActivity is a BaseActivity? This sounds inevitably wrong, because the very nature of a "base activity" is that there is just one such activity. A good example would be an EmployeeActivity inheriting from a PersonActivity, which could make sense, since an employee really is a person.

We don't have more details about your context. If there is no is-a relationship between the classes involved, then don't use inheritance, and prefer composition instead: let each class that needs a rest client get it from the singleton.

I agree with both @mjolka and @kyle-falconer. In fact the two points are very similar, because dependency injection frameworks usually work by giving you singletons by default. Mjolka's suggestion of adopting Dagger will be good long-term solution and definitely worth the time investment. Kyle's suggestion is more of a direct, simple solution you can implement right now.

If you go with Kyle's approach, I would point out the modern way of implementing singletons in Java using an enum, that's simpler and more robust:

enum SingletonRestClient {
 INSTANCE;
 private final RestClient restClient;
 private SingletonRestClient() {
 restClient = new RestAdapter.Builder().setEndpoint(BASE_URL).build().create(RestClient.class);
 }
 public RestClient getRestClient() {
 return restClient;
 }
}

You could use this in an activity like this:

public class SomeActivity extends Activity {
 private RestClient restClient;
 @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 restClient = SingletonRestClient.INSTANCE.getRestClient();
 }
}

Of course, you will have to do this in all your activities that need the rest client. But this is still better than forcing all your activities to extend a base activity.

Don't use inheritance just to share data. The right reason to use inheritance is when there is an is-a relationship. Can you say that SomeActivity is a BaseActivity? This sounds inevitably wrong, because the very nature of a "base activity" is that there is just one such activity. A good example would be an EmployeeActivity inheriting from a PersonActivity, which could make sense, since an employee really is a person.

We don't have more details about your context. If there is no is-a relationship between the classes involved, then don't use inheritance, and prefer composition instead: let each class that needs a rest client get it from the singleton.

Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

I agree with both @mjolka and @kyle-falconer. In fact the two points are very similar, because dependency injection frameworks usually work by giving you singletons by default. Mjolka's suggestion of adopting Dagger will be good long-term solution and definitely worth the time investment. Kyle's suggestion is more of a direct, simple solution you can implement right now.

If you go with Kyle's approach, I would point out the modern way of implementing singletons in Java using an enum, that's simpler and more robust:

enum SingletonRestClient {
 INSTANCE;
 private final RestClient restClient;
 private SingletonRestClient() {
 restClient = new RestAdapter.Builder().setEndpoint(BASE_URL).build().create(RestClient.class);
 }
 public RestClient getRestClient() {
 return restClient;
 }
}

You could use this in an activity like this:

public class SomeActivity extends Activity {
 private RestClient restClient;
 @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 restClient = SingletonRestClient.INSTANCE.getRestClient();
 }
}

Of course, you will have to do this in all your activities that need the rest client. But this is still better than forcing all your activities to extend a base activity.

Don't use inheritance just to share data. The right reason to use inheritance is when there is an is-a relationship. Can you say that SomeActivity is a BaseActivity? This sounds inevitably wrong, because the very nature of a "base activity" is that there is just one such activity. A good example would be an EmployeeActivity inheriting from a PersonActivity, which could make sense, since an employee really is a person.

We don't have more details about your context. If there is no is-a relationship between the classes involved, then don't use inheritance, and prefer composition instead: let each class that needs a rest client get it from the singleton.

lang-java

AltStyle によって変換されたページ (->オリジナル) /