2

I'm designing a class hierarchy and I come across the problem as my title, here is a smaller abstraction of it:

public abstract class A {
    private C c;

    public A(C c) {
        this.c = c;
    }

    // ...
}

public class B extends A {
    private int a;
    private int b;
    // ...

    public B(int a, int b, ... /* and more but no object of C */) {

        super(new C(this)); // <-- this is the point, got error:
                            //   can't access `this` .

        this.a = a;
        this.b = b;
        // ...
    }
}

public class C {
    private A a;

    public C(A a) {
        this.a = a;
    }
}

In short: A needs to keep a reference to C, vice versa, but both should be done in their constructor, and after the construction of both, the fields A a and C c should not be changed so I made them private. So in this situation what's the best practice?

The following is actually not a good example, but the point is that I have a situation need to do what the above try to do.

(More context: Now let's say I change private C c; of class A into 

private List<C> someCs;

that is: A is a container of C's, so during some situation those C's need to be sorted. And for some client of each C, the client needs to know what's its parent, i.e. which A it belongs to. In this situation I need references of both directions.)

NeoZoom.lua
  • 2,269
  • 4
  • 30
  • 64
  • 3
    I think you need to re-think your design decision, are you sure it is not enough to have the coupling in only one direction? Or at least only one needs to be set in the constructor? Also, it is better to use `final` for an instance variable that can't be changed. – Joakim Danielson May 23 '19 at 18:11
  • @JoakimDanielson: Let me provide more context. Updated. – NeoZoom.lua May 23 '19 at 18:13
  • There is an inherent circular dependency among your classes. Hence it is not a hierarchy anymore. We should see how we can remove that first. – Anindya Dutta May 23 '19 at 18:14
  • @AnindyaDutta: So this is in general a bad smell of code? – NeoZoom.lua May 23 '19 at 18:18
  • 1
    Given the update I see no need to set this in the constructor of either class, you create the list in A in the constructor but do you really no the content of the list then? More likely C needs a method `setParent` that gets called from within C (with `this` as argument) when a C object is added to the list. Come to think of it, why would an instance of A hold any other instances of C than those it is the parent of since the list is a instance variable in A? – Joakim Danielson May 23 '19 at 18:22
  • You could make the Constructor of C protected and give A a suitable Factory-method for creating new Cs that belong to A – Turo May 23 '19 at 18:30
  • @s̮̦̩e̝͓c̮͔̞ṛ̖̖e̬̣̦t̸͉̥̳̼, correct, irrespective of what this code is trying to achieve, a circular dependency is generally not a great way of making a code scalable, and can have weird behavior. Like them having the same instances being referenced, or references to other objects that can make bigger circular loops. – Anindya Dutta May 23 '19 at 18:32
  • `new C(this)` <-- You can't pass `this` to anything else because you're right in the middle of initializing it... that's what a constructor does. As Turo pointed out, you can separate out a constructor and a `static` factory method as a workaround (just remember to make the constructor private in order to force people to use the factory method). – Powerlord May 23 '19 at 18:37

1 Answers1

0

this is not a valid reference for other objects until after the constructor is done. for that reason this kind of cyclic dependency (parent with reference to child and child with reference to parent) cannot be initialized in the constructor. the parent child references also cannot be final because final must be initialized in the constructor.

you could write a factory that initializes all the parents and children. then after all are initialized you can cross reference them. but this needs child.parent and parent.child to be accessible to the factory. either they are public or have a public setter. you can do some other stunts to allow the factory to set a private variable in the parents and children but i do not recommend that.

here is one suggestion for your situation: create an object which tracks the mapping between parents and children. the mapping has method getParent(this) called from child when it needs its parent. and methods getChildren(this) and getChildrenSorted(this) called from parent when it needs its children.

the mapping can be initialized once and then locked down using private and final and whatnot preventing further changes during runtime. every parent and child can then get a private and final reference to this object.

for more information:

Lesmana
  • 25,663
  • 9
  • 82
  • 87