HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Functional interface hierarchy in Java 8

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
functionalinterfacehierarchyjava

Problem

I just created the following code and am wondering whether it is logically correct (all other feedback is of course also welcome):

```
public interface HandView extends View {
void onCardAdded(final Card card);

void onCardPlayed(final int cardIndex);

void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo);

public static HandView merge(final HandView... handViews) {
return new HandView() {
@Override
public void onCardAdded(final Card card) {
Arrays.stream(handViews).forEach(handView -> handView.onCardAdded(card));
}

@Override
public void onCardPlayed(final int cardIndex) {
Arrays.stream(handViews).forEach(handView -> handView.onCardPlayed(cardIndex));
}

@Override
public void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo) {
Arrays.stream(handViews).forEach(handView -> handView.onCardsSwapped(cardIndexOne, cardIndexTwo));
}
};
}

@FunctionalInterface
public static interface OnCardAddedListener extends HandView, Consumer {
@Override
default void accept(final Card card) {
onCardAdded(card);
}

@Override
void onCardAdded(final Card card);

@Override
default void onCardPlayed(final int cardIndex) { }

@Override
default void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo) { }
}

@FunctionalInterface
public static interface OnCardPlayedListener extends HandView, IntConsumer {
@Override
default void accept(final int cardIndex) {
onCardPlayed(cardIndex);
}

@Override
default void onCardAdded(final Card card) { }

@Override
void onCardPlayed(final int cardIndex);

@Override
default void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo) { }

Solution

Honestly, skiwi, you make things more complicated than they need to be.

Please rename View to Listener to avoid confusion. Because that's really what they are, it's a Listener, not a View. It doesn't by itself provide any UI functionality.

Prefixing your interfaces with On is, although understandable why you are doing it, not something I recommend or have seen been done in other places. You're already suffixing Listener to the name (which is what I would recommend, and what I have seen in lots of other code), no need to prefix the interface with On. The method names can be prefixed with on, as they have been already.

Why are you bringing IntConsumer and Consumer into your code? In what way does that help you? In my opinion, it doesn't help you at all. It creates more confusion than brings something good. If you want to "transform" an IntConsumer into a CardPlayedListener, use Java 8's method reference style by using something like IntConsumer::accept.

Now, let's see if we can write more or less the same thing of your interface heirarchy but with fewer lines.

public interface HandListener {
    default void onCardAdded(final Card card) {}

    default void onCardPlayed(final int cardIndex) {}

    default void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo) {}

    public static HandListener merge(final HandListener... handViews) {
        return new HandListener() {
            @Override
            public void onCardAdded(final Card card) {
                Arrays.stream(handViews).forEach(handView -> handView.onCardAdded(card));
            }

            @Override
            public void onCardPlayed(final int cardIndex) {
                Arrays.stream(handViews).forEach(handView -> handView.onCardPlayed(cardIndex));
            }

            @Override
            public void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo) {
                Arrays.stream(handViews).forEach(handView -> handView.onCardsSwapped(cardIndexOne, cardIndexTwo));
            }
        };
    }

    @FunctionalInterface
    public static interface CardAddedListener extends HandListener {
        @Override
        void onCardAdded(final Card card);
    }

    @FunctionalInterface
    public static interface CardPlayedListener extends HandListener {
        @Override
        void onCardPlayed(final int cardIndex);
    }

    @FunctionalInterface
    public static interface CardsSwappedListener extends HandListener {
        @Override
        void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo);
    }
}


This way, you can keep having only HandListeners as listeners in your Hand, no need to split it up into three separate listeners. And if you want to easily create a listener that listens only to cards being swapped, you can easily do so. The trick here is that I'm overriding a default method and removing the implementation, thus making it abstract again. While letting the original HandListener interface have default implementations for all the methods.

  • Original lines of code: 72



  • New lines of code: 43



  • Lines of code reduced by: 40%

Code Snippets

public interface HandListener {
    default void onCardAdded(final Card card) {}

    default void onCardPlayed(final int cardIndex) {}

    default void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo) {}

    public static HandListener merge(final HandListener... handViews) {
        return new HandListener() {
            @Override
            public void onCardAdded(final Card card) {
                Arrays.stream(handViews).forEach(handView -> handView.onCardAdded(card));
            }

            @Override
            public void onCardPlayed(final int cardIndex) {
                Arrays.stream(handViews).forEach(handView -> handView.onCardPlayed(cardIndex));
            }

            @Override
            public void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo) {
                Arrays.stream(handViews).forEach(handView -> handView.onCardsSwapped(cardIndexOne, cardIndexTwo));
            }
        };
    }

    @FunctionalInterface
    public static interface CardAddedListener extends HandListener {
        @Override
        void onCardAdded(final Card card);
    }

    @FunctionalInterface
    public static interface CardPlayedListener extends HandListener {
        @Override
        void onCardPlayed(final int cardIndex);
    }

    @FunctionalInterface
    public static interface CardsSwappedListener extends HandListener {
        @Override
        void onCardsSwapped(final int cardIndexOne, final int cardIndexTwo);
    }
}

Context

StackExchange Code Review Q#48783, answer score: 5

Revisions (0)

No revisions yet.