Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Fluent API with inheritance and generics

I'm writing a fluent API to configure and instantiate a series of "message" objects. I have a hierarchy of message types.

To be able to access method of subclasses when using the fluent API, I used generics to parametrize the subclasses and make all fluent methods (that start with "with") return the generic type. Note that I omitted most of the body of the fluent method; a lot of configuration goes on in them.

public abstract class Message<T extends Message<T>> {

    protected Message() {

    }

    public T withID(String id) {
        return (T) this;
    }
}

The concrete subclasses redefine the generic type similarly.

public class CommandMessage<T extends CommandMessage<T>> extends Message<CommandMessage<T>> {

    protected CommandMessage() {
        super();
    }

    public static CommandMessage newMessage() {
        return new CommandMessage();
    }

    public T withCommand(String command) {
        return (T) this;
    }
}

public class CommandWithParamsMessage extends
    CommandMessage<CommandWithParamsMessage> {

    public static CommandWithParamsMessage newMessage() {
        return new CommandWithParamsMessage();
    }

    public CommandWithParamsMessage withParameter(String paramName,
        String paramValue) {
        contents.put(paramName, paramValue);
        return this;
    }
}

This code works, i.e. I can instantiate any of the classes and use all fluent methods:

CommandWithParamsMessage msg = CommandWithParamsMessage.newMessage()
        .withID("do")
        .withCommand("doAction")
        .withParameter("arg", "value");

Calling the fluent methods in any order is a major goal here.

However, the compiler warns that all return (T) this are unsafe.

Type safety: Unchecked cast from Message to T

I'm unsure how I could reorganize the hierarchy to make this code truly safe. Even though it works, the use of generics in this fashion feels really convoluted. Especially, I'm not able to foresee situations where runtime exceptions will happen if I just ignore the warnings. There will be new message types, so I need to keep the code extensible. If the solution is to avoid inheritance altogether I would also like to obtain suggestion of alternatives.

There are other questions here on SO that address a similar issue. They point to a solution where all intermediate classes are abstract and declare a method like protected abstract self(). Still, in the end it's not safe.

like image 847
Boj Avatar asked May 19 '14 06:05

Boj


Video Answer


5 Answers

Your code is fundamentally an unsafe use of Generics. For example, if I write a new class which extends message, say Threat, and has a new method doSomething(), and then I create a message parameterised by this new class and it creates an instance of Message, and then attempts to Cast it to its subclass. However, since it is an instance of Message, and not of Threat, an attempt to call this message will cause an Exception. Since it is not possible for Message to doSOmething().

Further, its also unnecessary to use Generics here. Plain old inheritance will work fine. Since sub types can override methods by making their return types more specific, you can have:

public abstract class Message {

    protected Message() {

    }

    public Message withID(String id) {
        return this;
    }
}

And then

public class CommandMessage extends Message {

    protected CommandMessage() {
        super();
    }

    public static CommandMessage newMessage() {
        return new CommandMessage();
    }

    public CommandMessage withCommand(String command) {
        return this;
    }
}

This will work fine, on the understanding that you call your arguments in the right order:

CommandWithParamsMessage.newMessage()
    .withID("do")
    .withCommand("doAction")
    .withParameter("arg", "value");

will fail, but

CommandWithParamsMessage.newMessage().withParameter("arg", "value")
.withCommand("doAction").withID("do")

Will succeed, since it only "up types", finally returning a "message" class. If you want it not to "uptype", then simply overwrite the inherited commands, and now you can call the methods in any order, since they are all return the original type.

E.g.

public class CommandWithParamsMessage extends
CommandMessage {

    public static CommandWithParamsMessage newMessage() {
        return new CommandWithParamsMessage();
    }

    public CommandWithParamsMessage withParameter(String paramName,
        String paramValue) {
        contents.put(paramName, paramValue);
        return this;
    }

    @Override
    public CommandWithParamsMessage withCommand(String command){
        super.withCommand(command);
        return this;
   }

    @Override
    public CommandWithParamsMessage withID(String s){
        super.withID(s);
        return this;
    }
}

Now you will fluently return a CommandWithParamsMessage with either of the two fluent calls above.

Does this solve your problem, or have I misunderstood your intent?

like image 162
phil_20686 Avatar answered Oct 22 '22 20:10

phil_20686


I've done something like this before. It can get ugly. In fact, I've tried it more times than I've used it; usually it gets erased and I try to find a better design. That said, to help you move a little further down the road try this:

Have your abstract classes declare a method like:

protected abstract T self();

This can replace this in your return statements. The subclasses will be required to return something that matches the bound for T -- but it doesn't guarantee that they return the same object.

like image 35
William Price Avatar answered Oct 22 '22 19:10

William Price


If you change the signatures like this you should neither get any warnings nor do you need any casts:

abstract class Message<T extends Message<T>> {

    public T withID(String id) {
        return self();
    }

    protected abstract T self();
}

abstract class CommandMessage<T extends CommandMessage<T>> extends Message<T> {

    public T withCommand(String command) {
        // do some work ...
        return self();
    }
}

class CommandWithParamsMessage extends CommandMessage<CommandWithParamsMessage> {

    public static CommandWithParamsMessage newMessage() {
        return new CommandWithParamsMessage();
    }

    public CommandWithParamsMessage withParameter(String paramName, String paramValue) {
        // do some work ...
        return this;
    }

    @Override protected CommandWithParamsMessage self() {
        return this;
    }
}
like image 12
Harmlezz Avatar answered Oct 22 '22 20:10

Harmlezz


The compiler warns you of this unsafe operation, because it cannot factually check the correctness of your code. This makes it, as a matter of fact, unsafe and there is nothing you can do to prevent this warning. Even though an unsafe operation is not compile-checked, it can still be legitimate at run time. If you circumvent the compiler check, it is however your job to validate your own code for its use of correct types which is what the @SupressWarning("unchecked") annotation is for.

To apply this to your example:

public abstract class Message<T extends Message<T>> {

  // ...

  @SupressWarning("unchecked")
  public T withID(String id) {
    return (T) this;
  }
}

is fine, because you can as a matter of fact tell with certainty that this Message instance is always of the type that is represented by T. But the Java compiler cannot (yet). As with other suppression warnings, the key to using the annotation is to minimize its scope! Otherwise, you can easily retain the annotation suppression by accident after you made code changes that render your former manual check for type safety as invalid.

As you only return a this instance, you can however easily outsource the task to a specific methods as recommended in another answer. Define a protected method like

@SupressWarning("unchecked")
public T self() {
  (T) this;
}

and you can always call the mutator like here:

public T withID(String id) {
  return self();
}

As another option, and if it is possible for you to implement, consider an immutable builder which only exposes its API by interfaces but implements a full builder. This is how I normally build fluent interfaces these days:

interface Two<T> { T make() }
interface One { <S> Two<S> do(S value) }

class MyBuilder<T> implements One, Two<T> {

  public static One newInstance() {
    return new MyBuilder<Object>(null);
  }

  private T value; // private constructors omitted

  public <S> Two<S> do(S value) {
    return new MyBuilder<S>(value);
  }

  public T make() {
    return value;
  }
}

You can, of course, create smarter constructions where you avoid the unused fields. If you want to look at examples of me using this approach, look at my two projects which use fluent interfaces quite heavily:

  1. Byte Buddy: API for defining a Java class at run time.
  2. PDF converter: A conversion software for converting files using MS Word from Java.
like image 4
Rafael Winterhalter Avatar answered Oct 22 '22 21:10

Rafael Winterhalter


This is not a solution for your original problem. It is only an attempt to capture your actual intention, and sketch an approach where where the original problem does not appear. (I like generics - but class names like CommandMessage<T extends CommandMessage<T>> extends Message<CommandMessage<T>> make me shudder...)

I know that this is structurally rather different from what you originally asked about, and you might have omitted some details in the question that narrow down the range of possible answers so that the following is no longer applicable.

But if I understood your intention correctly, you could consider letting the subtypes be handled by the fluent calls.

The idea here is that you initially can only create a simple Message:

Message m0 = Message.newMessage();
Message m1 = m0.withID("id");

On this message, you can call the withID method - that's the only method that all messages have in common. The withID method returns a Message in this case.

Until now, the message is neither a CommandMessage nor any other specialized form. However, when you call the withCommand method, you obviously want to construct a CommandMessage - so you now simply return a CommandMessage:

CommandMessage m2 = m1.withCommand("command");

Similarly, when you call the withParameter method, you receive a CommandWithParamsMessage:

CommandWithParamsMessage m3 = m2.withParameter("name", "value");

This idea is roughly (!) inspired by a blog entry, which is in German, but the code nicely shows how this concept may be used to construct type-safe "Select-From-Where" queries.

Here, the approach is sketched, roughly adapted for your use-case. Of course, there are some details where the implementation will depend on how this is actually going to be used - but I hope that the idea becomes clear.

import java.util.HashMap;
import java.util.Map;


public class FluentTest
{
    public static void main(String[] args) 
    {
        CommandWithParamsMessage msg = Message.newMessage().
                withID("do").
                withCommand("doAction").
                withParameter("arg", "value");


        Message m0 = Message.newMessage();
        Message m1 = m0.withID("id");
        CommandMessage m2 = m1.withCommand("command");
        CommandWithParamsMessage m3 = m2.withParameter("name", "value");
        CommandWithParamsMessage m4 = m3.withCommand("otherCommand");
        CommandWithParamsMessage m5 = m4.withID("otherID");
    }
}

class Message 
{
    protected String id;
    protected Map<String, String> contents;

    static Message newMessage()
    {
        return new Message();
    }

    private Message() 
    {
        contents = new HashMap<>();
    }

    protected Message(Map<String, String> contents) 
    {
        this.contents = contents;
    }

    public Message withID(String id) 
    {
        this.id = id;
        return this;
    }

    public CommandMessage withCommand(String command) 
    {
        Map<String, String> newContents = new HashMap<String, String>(contents);
        newContents.put("command", command);
        return new CommandMessage(newContents);
    }

}

class CommandMessage extends Message 
{
    protected CommandMessage(Map<String, String> contents) 
    {
        super(contents);
    }

    @Override
    public CommandMessage withID(String id) 
    {
        this.id = id;
        return this;
    }

    public CommandWithParamsMessage withParameter(String paramName, String paramValue) 
    {
        Map<String, String> newContents = new HashMap<String, String>(contents);
        newContents.put(paramName, paramValue);
        return new CommandWithParamsMessage(newContents);
    }

}

class CommandWithParamsMessage extends CommandMessage 
{
    protected CommandWithParamsMessage(Map<String, String> contents) 
    {
        super(contents);
    }

    @Override
    public CommandWithParamsMessage withID(String id) 
    {
        this.id = id;
        return this;
    }

    @Override
    public CommandWithParamsMessage withCommand(String command) 
    {
        this.contents.put("command", command);
        return this;
    }
}
like image 3
Marco13 Avatar answered Oct 22 '22 19:10

Marco13