Constructor in abstract and non-abstract classes

Question

I have a GenericContainer class and a FIFOContainer class that extends the generic one. My problem appears when trying to use the takeout() method. It does not recognize that I hold values in my FIFOContainer ArrayList. I suspect this has something to do with how I defined the constructors but for the life of me cannot figure it out how to solve it.

A solution I thought of is defining a getter in the GenericContainer class and passing the value in the FIFOContainer class but I feel like this should not be needed.

public abstract class GenericContainer implements IBag {

    private ArrayList<ISurprise> container;

    public GenericContainer() {
        this.container = new ArrayList<ISurprise>();
    }

    @Override
    public void put(ISurprise newSurprise) {
        this.container.add(newSurprise);
    }

    @Override
    public void put(IBag bagOfSurprises) {
        while (!bagOfSurprises.isEmpty()) {
            System.out.println(bagOfSurprises.size());
            this.container.add(bagOfSurprises.takeout());
        }
    }

    @Override
    public boolean isEmpty() {
        if (this.container.size() > 0) {
            return false;
        }
        return true;
    }

    @Override
    public int size() {
        if (isEmpty() == false) {
            return this.container.size();
        }
        return -1;
    }

}

public class FIFOContainer extends GenericContainer {

    private ArrayList<ISurprise> FIFOcontainer;

    public FIFOContainer() {
        super();
        this.FIFOcontainer = new ArrayList<ISurprise>();        

    }

    public ISurprise takeout() {
        if (isEmpty() == false) {
            this.FIFOcontainer.remove(0);
            ISurprise aux = this.FIFOcontainer.get(0);
            return aux;
        }
        return null;
    }
}

You should follow the Java Naming Conventions: variable names and method names are written in camelCase. So `FIFOcontainer` violates these conventions. — MC Emperor, May 09 '19 at 13:42
Another hint: your size() method, that is really nonsensical. Again: there is **zero** value in that if statement. The GenericContainer size ... is the size of its container. You are writing a lot of code that is just *not* necessary. Worse: it is confusing, and thus allows for bugs to hide. as: no java collection can have a size of -1. Sizes go from 0 to whatever. They are NOT negative. — GhostCat, May 09 '19 at 14:01

GhostCat · Accepted Answer · 2019-05-09T13:55:35.930

1

Thing is: fields are not poylmorphic (see here for example).

Your problem: basically your isEmpty() will use the container in the base class, and the other method will us the container in the subclass.

Yes, there are two containers in your class.

A better approach could be to (for example) do this in the base class GenericContainer:

protected abstract List<ISurprise> getContainer();

In other words: a subclass could provide its own container, and base methods as isEmpty() could use that one:

@Override
public final boolean isEmpty() {
    return getContainer().isEmpty();
}

To allow for more degrees of freedom, that method could also have a slightly different signature, such as protected abstract Collection<ISurprise> to be more flexible about the actual implementation.

( hint: I made the method final, as that is the whole idea of methods defined in an abstract base class: that subclasses do not overwrite them )

( and bonus hints: try to minimize the amount of code that you write. you don't do someBool == true/false, you don't need to do getSize() == 0 when that list class already offers you an isEmpty() method )

edited May 09 '19 at 13:55

answered May 09 '19 at 13:17

GhostCat

137,827
25
176
248

Instead of overriding the isEmpty() method in the FIFOcontainer class, wouldn't if (getContainer().isEmpty() == false) work just as well? – Bogdan May 09 '19 at 13:42
@BogdanBuzaianu please read carefully: my approach would be: the base class can call `getContainer()` to do its job, and the **child** classes would implement that. And again: when you already have a method that goes `boolean isEmpty()` on that collection itself, then putting an if statement around that and == false is nothing but **noise**. The result of base.isEmpty() is just that: getContainer.getEmpty(). No need for an if, no need for a comparison, no need to say == false, no need for a second return true. – GhostCat May 09 '19 at 13:57
This is where i get confused: I am defining the abstract method getContainer in the GenericContainer class. I am overriding it in the FIFOContainer class. But then I'm doing this in the GenericContainer isEmpty(): return getContainer.isEmpty();. Since the method is overriden in the child class, how should it work? Sorry, I am really trying to understand but OOP is pretty new for me – Bogdan May 09 '19 at 14:23
@BogdanBuzaianu The child class implements "getContainer()". Only that. The base class implements isEmpty(). And the child class doesn't touch isEmpty()! – GhostCat May 09 '19 at 14:28
Which means I do not need an ArrayList field in the base class anymore, since I can always call getContainer()? – Bogdan May 09 '19 at 14:40
@BogdanBuzaianu Exactly. That is one option. Of course, you could have a list in the base class, but then **only** in the base class. And then you would probably make that one protected. From a conceptual point of view, both options could work. It really depends on your exact requirements, and the underlying goal you intend to achieve. But please understand that SO is not a discussion forum. You ask a question, you get an answer. Coming back with more and more questions within comments isn't how things should go here. – GhostCat May 09 '19 at 14:45

score 0 · Answer 2 · answered May 09 '19 at 13:12

0

You shouldn't create new arraylist in FIFOContianer. you already have list from GenericContainer.
So right now, when you are calling put it adds item to the container list in the parent class. when calling takeout you are accessing other list (FIFOcontainer) which you created in the child class.

just remove FIFOcontainer and keep using container:

 public ISurprise takeout() {
        if (isEmpty() == false) {
            this.container.remove(0);
            ISurprise aux = this.container.get(0);
            return aux;
        }
        return null;
    }

answered May 09 '19 at 13:12

matanper

881
8
24

But the container field in GenericContainer is private. This would not work without a getter – Bogdan May 09 '19 at 13:18
you should change it to `protected` – matanper May 09 '19 at 13:21
@BogdanBuzaianu When the getter is *protected* then of course it can *get* you a private field of a superclass. But as I have written in my answer: that is the wrong way. I *might* make more sense for the subclasses to give access to "their" containers. – GhostCat May 09 '19 at 13:41
@GhostCat I agree you're answer is the better one, and it good to notice that it follows the Open-Closed principle – matanper May 09 '19 at 13:55

Constructor in abstract and non-abstract classes

2 Answers2