Composition of two classes with common inheritance root

Question

Hope your day is going well.

I want to make the following composition of two objects with the same inheritance root:

There will be 3 basic operations on the Wallet class: add, getCards and getPaymentCards. getCards should return both Cards and PaymentCards. This is a place where I've faced a problem of how to store these cards in the Wallet:

class Card {}
class BadgeCard extends Card {}
class IdCard extends Card {}

class PaymentCard extends Card {}

class WalletT21 {
    Set<Card> cards;
    Set<PaymentCard> paymentCards;

    void add(Card c) {
        cards.add(c);
        Integer i = 0;
        i = 1;
        i = 1;
        System.out.println(i);
    }

    void add(PaymentCard c) {
        paymentCards.add(c);
    }

    Set<Card> cards() {
        return Stream.concat(cards.stream(), paymentCards.stream())
                     .collect(Collectors.<Card>toSet());
    }

    Set<PaymentCard> paymentCards() {
        return paymentCards;
    }
}
class WalletT22 {
    Set<Card> cards;
    Set<PaymentCard> paymentCards;

    void add(Card c) {
        cards.add(c);
    }

    void add(PaymentCard c) {
        cards.add(c);
        paymentCards.add(c);
    }

    Set<Card> cards() {
        return cards;
    }

    Set<PaymentCard> paymentCards() {
        return paymentCards;
    }
}

All these ideas are, let's say, not perfect. The reason for uniting these classes under the one roof (Wallet) is that the higher code uses cards for identification (every card is an account identifier) and payments (not every card is payable but every paymentcard is an identifier (Card)).

Is there a more elegant way to make such composition?

Answer 1

All cards could be stored in a single Set<Card>, and then in getPaymentCards a filter by type could be applied:

cards.stream()
    .filter(card -> card instanceof PaymentCard)
    .collect(Collectors.toSet());

The problem should probably be modelled differently to avoid this problem entirely, but given these exact requirements this solution might be a cleaner way to handle it.

If a more generic version was needed, this could be implemented like so:

public Set<Card> getCardsOfType(Class<? extends Card> type) {
    return cards.stream()
            .filter(type::isInstance)
            .collect(Collectors.toSet());
}

Answer 2

Edit: Thanks to @Laiv and @BenAaronson, here I use generics instead of an actual instance to pass to the filter method. See edit history for the previous version of the code.

Well, I don't like using instanceOf because if would need adding new special cases as new types of cards arise. Also your Wallet class is coupled to concrete implementations.

So here's my two cents:

getCardsByType(Class<? extends Card> cardType)

You pass something like MyIDCard.class to the method to get a subset of all cards of that type . The consumer then can cast the cards to the type he wants if needed. I use interfaces but you could use abstract classes if you like, it would work the same.

Note that only the test class is coupled to any concrete Card implementation, which in a real app should be the factory.

Show me the code (test method at the end):

import java.util.Set;
import java.util.HashSet;
import java.util.Set;

interface Card {}
interface Badge extends Card {}
interface IDCard extends Card {}
interface PaymenCard extends Card {}
class MyIDCard implements IDCard {}
class MyBadge implements Badge {}

interface Wallet {
    public void addCard(Card card);
    public Set<Card> getCards();
    public Set<Card> getCardsByType(Class<? extends Card> cardType);
}

class MyWallet implements Wallet {
    Set<Card> cards = new HashSet<Card>();
    @Override
    public void addCard(Card card) { this.cards.add(card);}
    @Override
    public Set<Card> getCards() { return this.cards;}
    @Override
    public Set<Card> getCardsByType(Class<? extends Card> cardType) {
        Set<Card> subSet = new HashSet<Card>();         
        for (Card card: this.cards){
            if (card.getClass() == cardType){
                subSet.add(card);   
            }           
        }
        return subSet;
    }
}

---

    public class Test {    
        public static void main (String[] args){
            Card c1 = new MyIDCard();
            Card c2 = new MyBadge();
            Card c3 = new MyBadge();
            Card c4 = new MyBadge();
            Card c5 = new MyIDCard();
            Wallet myWallet = new MyWallet();
            myWallet.addCard(c1);
            myWallet.addCard(c2);
            myWallet.addCard(c3);
            myWallet.addCard(c4);
            myWallet.addCard(c5);
            //now my wallet has two IDs and a Badge
            //
            // I can ask for a list of cards "like this one"
            Set<Card> badges = myWallet.getCardsByType(MyBadge.class); 

            for (Card card: badges){
                System.out.println(card.getClass().getCanonicalName());
            }       
        }
    }

---

Output showing that the filter by type works:

$ java Test 
MyBadge
MyBadge
MyBadge

Here I use a default package, but no name colission is possible since canonical name of classes contains the package namespace like this:

$ java Test 
com.test.cards.MyBadge
com.test.cards.MyBadge
com.test.cards.MyBadge

Answer 3

As other people mentioned having them all in a single Set is the best option, however if you absolutely must have 2 separate Sets for whatever reason you can simply have getCards() return a combination of the Sets.

void add(Card c) {
    cards.add(c);
}

void add(PaymentCard c) {
    paymentCards.add(c);
}

Set<Card> cards() {
    Set<Card> combinedSet = new HashSet<>();
    combinedSet.addAll(cards);
    combinedSet.addAll(paymentCards);
    return combinedSet;
}

Set<PaymentCard> paymentCards() {
    return paymentCards;
}