Networking layer module

Question

These two classes are part of a networking layer module that I am building:

public abstract class BaseRequest {
 protected RequestCreator mCreator;
 public BaseRequest() {
 setupRequestCreator();
 }
 private setupRequestCreator() {
 mCreator = new RequestCreator.Builder()
 .headers(getHeaders())
 .parser(getParser())
 .build();
 }
 public abstract Headers getHeaders();
 public abstract Parser getParser();
}
public class UserRequest extends BaseRequest {
 private Headers mHeaders;
 private Request<User> mRequest;
 private interface UserService {
 @GET("/users/{id}")
 User getUser(String id);
 }
 public UserRequest(Headers headers) {
 mHeaders = headers;
 mRequest = mCreator.createRequest(UserService.class);
 }
 @Override
 public Headers getHeaders() {
 return mHeaders;
 }
 @Override
 public Parser getParser() {
 return null;
 }
}

The framework that I am using requires me to configure/build a RequestCreator for each type of Request that I am going to use.

To make the process of adding new classes for different requests as pain-free as possible, I moved the building logic of the RequestCreator in an abstract class. This way I'll only have to extend it and override some methods that indicate how the RequestCreator should be configured - sort of a template method pattern.

The actual problem is this: when someone creates an instance of UserRequest, super() gets called right before UserRequest's member variables are set. What this means is that the RequestCreator will already be built by the time mHeaders are set. This is wrong because the building process needs a reference to mHeaders in order for it to behave as intended.

One way to get around this is to leave BaseRequest's constructor empty and to call setupRequestCreator() in the UserRequest's constructor, but only after setting the instance variables first. This however feels "hacky" because other people will have to be aware of this when they are creating new subclasses.

I tried other workarounds too, but nothing felt quite right. I was hoping someone could provide me with a better solution.

Answer 1

Why are you using an abstract class in the first place?

I more think you should use a factory method. You could store the headers and the parser in a class, but make it an immutable class.

Another note, or maybe just my personal opinion: It is to my knowledge not a Java convention to prefix all member fields with m.

The only really valuable code that you have is this:

mCreator = new RequestCreator.Builder()
 .headers(getHeaders())
 .parser(getParser())
 .build();

and

mRequest = mCreator.createRequest(UserService.class);

So let's make factory methods!

public class Requests {
 public static RequestCreator newCreator(Headers headers, Parser parser) {
 mCreator = new RequestCreator.Builder()
 .headers(headers)
 .parser(parser)
 .build();
 }
}
public class UserRequest {
 private interface UserService {
 @GET("/users/{id}")
 User getUser(String id);
 }
 public static Request<User> createUserRequest(Headers headers) {
 return Requests.newCreator(headers, null).createRequest(UserService.class);
 }
}

Problem solved(?)

• Alexandru Pele: Thank you Simon! Prefixing member fields with m is not indeed a Java convention but this code is part of an Android app and this is a very common practice in Android. Google uses this convention it in their own Android classes and I guess other developers just stick with it. Your approach using factory methods is also a lot more flexible and it suits my problem quite well!