Usage of multiple inheritance in Java 8

问题:

Am I using a feature of Java 8 or misusing it?

Refer the code and explanation below to know as to why it was chosen to be like this.

public interface Drawable {
    public void compileProgram();

    public Program getProgram();

    default public boolean isTessellated() {
        return false;
    }

    default public boolean isInstanced() {
        return false;
    }

    default public int getInstancesCount() {
        return 0;
    }

    public int getDataSize();

    public FloatBuffer putData(final FloatBuffer dataBuffer);

    public int getDataMode();

    public boolean isShadowReceiver();

    public boolean isShadowCaster();    //TODO use for AABB calculations

    default public void drawDepthPass(final int offset, final Program depthNormalProgram, final Program depthTessellationProgram) {
        Program depthProgram = (isTessellated()) ? depthTessellationProgram : depthNormalProgram;
        if (isInstanced()) {
            depthProgram.use().drawArraysInstanced(getDataMode(), offset, getDataSize(), getInstancesCount());
        }
        else {
            depthProgram.use().drawArrays(getDataMode(), offset, getDataSize());
        }
    }

    default public void draw(final int offset) {
        if (isInstanced()) {
            getProgram().use().drawArraysInstanced(getDataMode(), offset, getDataSize(), getInstancesCount());
        }
        else {
            getProgram().use().drawArrays(getDataMode(), offset, getDataSize());
        }
    }

    default public void delete() {
        getProgram().delete();
    }

    public static int countDataSize(final Collection<Drawable> drawables) {
        return drawables.stream()
                .mapToInt(Drawable::getDataSize)
                .sum();
    }

    public static FloatBuffer putAllData(final List<Drawable> drawables) {
        FloatBuffer dataBuffer = BufferUtils.createFloatBuffer(countDataSize(drawables) * 3);
        drawables.stream().forEachOrdered(drawable -> drawable.putData(dataBuffer));
        return (FloatBuffer)dataBuffer.clear();
    }

    public static void drawAllDepthPass(final List<Drawable> drawables, final Program depthNormalProgram, final Program depthTessellationProgram) {
        int offset = 0;
        for (Drawable drawable : drawables) {
            if (drawable.isShadowReceiver()) {
                drawable.drawDepthPass(offset, depthNormalProgram, depthTessellationProgram);
            }
            offset += drawable.getDataSize();   //TODO count offset only if not shadow receiver?
        }
    }

    public static void drawAll(final List<Drawable> drawables) {
        int offset = 0;
        for (Drawable drawable : drawables) {
            drawable.draw(offset);
            offset += drawable.getDataSize();
        }
    }

    public static void deleteAll(final List<Drawable> drawables) {
        drawables.stream().forEach(Drawable::delete);
    }
}

---

public interface TessellatedDrawable extends Drawable {
    @Override
    default public boolean isTessellated() {
        return true;
    }
}

---

public interface InstancedDrawable extends Drawable {
    @Override
    default public boolean isInstanced() {
        return true;
    }

    @Override
    public int getInstancesCount();
}

---

public class Box implements TessellatedDrawable, InstancedDrawable {
    //<editor-fold defaultstate="collapsed" desc="keep-imports">
    static {
        int KEEP_LWJGL_IMPORTS = GL_2_BYTES | GL_ALIASED_LINE_WIDTH_RANGE | GL_ACTIVE_TEXTURE | GL_BLEND_COLOR | GL_ARRAY_BUFFER | GL_ACTIVE_ATTRIBUTE_MAX_LENGTH | GL_COMPRESSED_SLUMINANCE | GL_ALPHA_INTEGER | GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH | GL_ALREADY_SIGNALED | GL_ANY_SAMPLES_PASSED | GL_ACTIVE_SUBROUTINE_UNIFORM_MAX_LENGTH | GL_ACTIVE_PROGRAM | GL_ACTIVE_ATOMIC_COUNTER_BUFFERS | GL_ACTIVE_RESOURCES | GL_BUFFER_IMMUTABLE_STORAGE;
        int KEEP_OWN_IMPORTS = UNIFORM_PROJECTION_MATRIX.getLocation() | VS_POSITION.getLocation();
    }
//</editor-fold>
    private FloatBuffer data;
    private Program program;

    private final float width, height, depth;

    public Box(final float width, final float height, final float depth) {
        this.width = width;
        this.height = height;
        this.depth = depth;
        data = generateBox();
        data.clear();
    }

    @Override
    public void compileProgram() {
        program = new Program(
                new VertexShader("data/shaders/box.vs.glsl").compile(),
                new FragmentShader("data/shaders/box.fs.glsl").compile()
        ).compile().usingUniforms(
                        UNIFORM_MODEL_MATRIX,
                        UNIFORM_VIEW_MATRIX,
                        UNIFORM_PROJECTION_MATRIX,
                        UNIFORM_SHADOW_MATRIX
                        );
    }

    @Override
    public int getInstancesCount() {
        return 100;
    }

    @Override
    public Program getProgram() {
        return program;
    }

    @Override
    public int getDataSize() {
        return 6 * 6;
    }

    @Override
    public FloatBuffer putData(final FloatBuffer dataBuffer) {
        FloatBuffer returnData = dataBuffer.put(data);
        data.clear();   //clear to reset data state
        return returnData;
    }

    @Override
    public int getDataMode() {
        return GL_TRIANGLES;
    }

    @Override
    public boolean isShadowReceiver() {
        return true;
    }

    @Override
    public boolean isShadowCaster() {
        return true;
    }

    private FloatBuffer generateBox() {
        FloatBuffer boxData = BufferUtils.createFloatBuffer(6 * 6 * 3);

        //put data into boxData

        return (FloatBuffer)boxData.clear();
    }
}

First the steps on how I came to this code:

1. I started with the Drawable interface and each implementation having its own drawDepthPass, draw and delete methods.

2. Refactoring delete to a default method was easy, trivial and should not be wrong.

3. However to be able to refactor drawDepthPass and draw I needed access to whether a Drawable was tesselated and/or instanced, so I added the public (non-default) methods isTessellated(), isInstanced() and getInstancesCount().

4. Then I figured out it would be slightly cumbersome, as we programmers are lazy, to implement them in every Drawable.

5. As a consequence I added the default methods to Drawable, giving the behaviour of the most basic Drawable.

6. Then I figured that I am still lazy and do not want to manually implement it for the tessellated and instanced variants eithere.

7. So I created TessellatedDrawable and InstancedDrawable that provide default isTessellated() and isInstanced() respectively. And in InstancedDrawable I revoked the default implementation of getInstancesCount().

As a result I can have the following:

• Normal Drawable: public class A implements Drawable
• Tessellated Drawable: public class A implements TessellatedDrawable
• Instanced Drawable: public class A implements InstancedDrawable
• Tessellated and instanced Drawable: public class A implements InstancedDrawable, TessellatedDrawable.

Just to ensure you, this all compiles and runs fine, the implements InstancedDrawable, TessellatedDrawable gets handled perfectly by Java 8 as there is nowhere ever ambiguity on from which interface the functionality should come.

Now onto my own little OOP design assessment:

• Every Drawable is in fact a Drawable, so Collection<Drawable> will not break.
• It is possible to group all TessellatedDrawable and/or InstancedDrawable, irrelevant of how exactly it is implement.

Other thoughts I had:

• Use a more conventional, layered approach, however I disregarded that as it would end up in:

• abstract class AbstractDrawable

• class Drawable extends AbstractDrawable
• class TessellatedDrawable extends AbstractDrawable
• class InstancedDrawable extends AbstractDrawable
• class InstancedTessellatedDrawable extends AbstractDrawable

I have also considered a Builder Pattern, however that is a pattern to be used when you are creating a lot of unique instances of a certain object, and that is not what we are doing here, neither is this about the constructor of the object.

So the first and final question was: Am I using a feature of Java 8 or misusing it?

回答1:

First, if it works, and it does what you want to do, and there's no danger of something breaking in the future, it doesn't make sense to say you're misusing it. After all, it got the job done, right? Features like default methods and static methods were added to interfaces with particular goals in mind, but if they help you achieve other goals, either that's creative use of a new feature or a gross and dirty hack. :-) To a certain extent it's a matter of taste.

With that perspective in mind, what I look for in APIs, and what I try to do when designing APIs, is to distinguish clients of an API from implementors of an API. A typical client, or user, of an API gets a reference of some interface type from somewhere and calls methods on it to make stuff happen. An implementor provides implementations for methods defined in interfaces, overrides methods, and (if subclassing) calls superclass methods. Often, the methods intended to be called by clients are different from those intended to be called from subclasses.

It seems to me that these concepts are being mixed in the Drawable interface. Certainly, clients of a Drawable will do things like call the draw or drawDepthPass methods on them. Great. But looking at the default implementation of drawDepthPass, it gets some information using isTessellated and isInstanced methods, and then uses these to choose a Program and call methods on it in a particular way. It's fine for these bits of logic to be encapsulated within a method, but in order for it to be done in a default method, the getters have to be forced into the public interface.

I might be wrong about your model, of course, but it seems to me that this kind of logic is more suited for an abstract superclass and subclasser relationship. The abstract superclass implements some logic that handles all Drawables, but it negotiates with the particular Drawable implementations with methods like isTesselated or isInstanced. In an abstract superclass, these would be protected methods that subclasses are required to implement. By putting this logic into default methods of an interface, all of these have to be public, which clutters up the client interface. The other methods that seem similar are getDataMode, isShadowReceiver, and isShadowCaster. Are clients expected to call these, or are they logically internal to the implementation?

What this highlights is that, despite the addition of default methods and static methods, interfaces are still oriented toward clients, and less toward supporting subclasses. The reasons are as follows:

• Interfaces have only public members.
• Abstract classes can have protected methods for subclasses to override or call.
• Abstract classes can have private methods to enable implementation sharing.
• Abstract classes can have fields (state) which can be protected to share state with subclasses, or usually private otherwise.
• Abstract classes can have final methods that enforce certain behavior policies on subclasses.

Another issue I note with the Drawable interface family is that it uses the ability of default methods to override each other to allow some simple mixins to the implementation classes like Box. It is kind of neat that you can just say implements TessellatedDrawable and avoid the pesky overriding of the isTesselated method! The problem is that this now becomes part of the implementation class's type. Is it useful for the client to know that a Box is also a TessellatedDrawable? Or is this just a scheme for making the internal implementation cleaner? If it's the latter, it might be preferable these mixin interfaces like TessellatedDrawable and InstancedDrawable not be public interfaces (i.e., package private).

Note also that this approach clutters up the type hierarchy, which can make code more confusing to navigate. Usually a new type is a new concept, but it seems heavyweight to have interfaces that merely define default methods returning boolean constants.

A further point in this vein. Again, I don't know your model, but the characteristics being mixed in here are very simple: they're just boolean constants. If there's ever a Drawable implementation that, say, starts off not being instanced and later can become instanced, it can't use these mixin interfaces. The default implementations are really quite restricted in what they can do. They can't call private methods or inspect fields of an implementation class, so their use is quite limited. Using interfaces this way is almost like using them as marker interfaces, with a tiny addition of being able to call a method to get the characteristic, instead of using instanceof. There doesn't seem to be much use beyond this.

The static methods in the Drawable interface seem mostly reasonable. They're utilities that seem client-oriented, and they provide reasonable aggregations of logic provided by the public instance methods.

Finally, there are a few points about the model that seem odd, though they're not directly related to the use of default and static methods.

It seems like a Drawable has-a Program, as there are instance methods compileProgram, getProgram, and delete. Yet the drawDepthPass and similar methods require the client to pass in two programs, one of which is selected depending on the result of the boolean getters. It's not clear to me where the caller is supposed to choose the right Programs.

Something similar is going on with the drawAll methods and the offset value. It seems like in a list of Drawables, they have to be drawn using particular offsets based on each Drawable's data size. Yet what is apparently the most fundamental method, draw, requires the caller to pass in an offset. This seems like a big responsibility to push onto the caller. So perhaps the offset stuff really belongs within the implementation as well.

There are a couple methods that take a List of drawables and call stream() and then forEach() or forEachOrdered(). This isn't necessary, as List has a forEach method on it, inherited from Iterable.

I think it's great to explore how this new stuff can be used. It's new enough that a commonly accepted style hasn't yet emerged. Experiments like this, and this discussion, help to develop that style. On the other hand, we also need to be careful not to use these shiny new features just because they're new and shiny.