Is using composition to leverage inheritance from Java abstract collections classes bad practice?

Question

A lot of times I find myself (what appears to be) combining composition and inheritance when inheriting from abstract collection classes and I can't help but feel like it's technically conflicting design paradigms while I do it.

/** Custom String List implementation */
public class MyStringList extends AbstractList<String>
{
    private final AbstractList<String> _myStringList;

    public MyStringList()
    {
        this._myStringList = new ArrayList<String>();
    }

    public MyStringList(Collection<String> stringCollection)
    {
        if (stringCollection.contains(null))
        {
            throw new NullPointerException("Null values not permitted in MyStringList");
        }
        else
        {
            this._myStringList = new ArrayList<String>(stringCollection);
        }
    }

    @Override
    public String get(int index)
    {
        return this._myStringList.get(index);
    }

    @Override
    public int size()
    {
        return this._myStringList.size();
    }

    @Override
    public String set(int index, String aString)
    {
        ++this.modCount;
        return this._myStringList.set(index, Objects.requireNonNull(aString));
    }

    @Override
    public void add(int index, String aString)
    {
        ++this.modCount;
        this._myStringList.add(index, Objects.requireNonNull(aString));
    }

    @Override
    public boolean add(String aString)
    {
        ++this.modCount;
        return this._myStringList.add(Objects.requireNonNull(aString));
    }

    @Override
    public String remove(int index)
    {
        ++this.modCount;
        return this._myStringList.remove(index);
    }

    // Method to make this list implementation distinct from ArrayList for the sake of
    // this StackOverflow question example
    public String[] getNumericContaining()
    {
        return this._myStringList.stream()
                                 .filter(aString -> aString.codePoints().anyMatch(Character::isDigit))
                                 .toArray(String[]::new);
    }

    // Another method to make this list implementation distinct from ArrayList for
    // the sake of this StackOverflow question example
    public String[] getUpperCaseContaining()
    {
        return this._myStringList.stream()
                                 .filter(aString -> aString.codePoints().anyMatch(Character::isUpperCase))
                                 .toArray(String[]::new);
    }
}

Is this design of having an internal abstract collection object being the backing object (composition) of the class this object inherits from (inheritance) considered the "correct" way of leveraging the various abstract collection classes defined in the java.util package?

Answer 1

You're combining things unnecessarily. If you want to have what you have in the example, go with Lino's suggestion:

public class MyStringList extends ArrayList<String> {

    public String[] getNumericContaining() {
        return this.stream()
                   .filter(aString -> aString.codePoints().anyMatch(Character::isDigit))
                   .toArray(String[]::new);
    }

    public String[] getUpperCaseContaining() {
        return this.stream()
                   .filter(aString -> aString.codePoints().anyMatch(Character::isUpperCase))
                   .toArray(String[]::new);
    }
}

however many times a regular List<String> is all you need, since you can extract extra behaviour to the class that encapsulates the list, e.g.

public class MySomeService {
    
    // This could be passed in, loaded from somewhere, etc.
    private List<String> foo;

    public void serviceMethod() {
        doSomething();
        String[] numbers = getNumericContaining();
        doSomethingElse(numbers);
    }

    // The previous methods
    private String[] getNumericContaining() {
        foo.stream(). // and so on
}

It depends on whether your list really is a class in its own right, or whether it's just a special flavor that you need at some point. If it's the first, it may deserve its own class, if it's the latter, it doesn't.

Finally, even though OOP tells you to put data and behaviour in the same place, sometimes it makes sense to keep them in different places, so a method or a function can be used. Just pass the list as a parameter, and it'll work for all Collection classes not just your special one.