9

I have two java class files. Each of them has methods the other one uses.

public class class1{
    class2 c2 = new class2(); 
    m1(){
        c2.ma(); 
        m2();
    }
    m2(){}
}

public class class2{
    class1 c1 = new class1();
    ma(){}
    mb(){
        c1.m2();
    }
}

The lines 

class1 c1 = new class1();

and 

class2 c2 = new class2();

refer to each other causing an infinite loop, resulting in a java.lang.StackOverflowError error.

Is there some way to have the classes refer to each other or do I have no choice but to transfer all of my methods into a single class?

Improve this question
5
  • 1
    Is this some kind of a practice program? If not, the cyclic references could probably be a code-smell. Do you really need both classes to depend on each other? Commented Jun 28, 2015 at 14:16
  • Check this pattern: en.wikipedia.org/wiki/Inversion_of_control Commented Jun 28, 2015 at 14:29
  • 1
    A stack overflow has nothing to do with classes referencing each other. It is perfectly possible and supported by Java to do that. The Exception is caused by a too deep recursin, this can happen with one method/class as well. Commented Jun 28, 2015 at 14:30
  • 1
    A stack overflow has nothing to do with classes referencing each other. Not sure what you mean. The StackOverflowError is due to a a cyclic constructor call. Commented Jun 28, 2015 at 14:30
  • Exactly what I am saying, a StackOverflowError happens if you are to deep in the call stack. It is a runtime error and has nothing to do with the static class layout. In this specific example I guess it is caused by initializing even more classes. Having one of both static could help. Commented Jun 28, 2015 at 14:31

3 Answers 3

Reset to default
6

As is said above, this is a sign of code smell.

Having a setter to set the method afterwards is not satisfactory, as you have an object in an indeterminate state, until the setters are called.

Although using a dependency framework such as Spring can help solve the above problem, if you use constructor injection, then you cannot have cyclic dependencies either! But at least when a bean is injected, you are sure it is not half constructed.

If you don't want to use a dependency injection framework, consider a factory pattern where both objects are created by a factory method, which returns a tuple (or a container object in the case of Java which has no native support for tuples) containing the fully constructed objects.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

An excellent answer. 1+. I'm deleting my answer in favor of this one.
@HovercraftFullOfEels Just an aggregation of our answers. :)
@ChetanKinger: perhaps, but both your answers are better than mine. Mine got more votes for being the first, but it's wrong.
@ChetanKinger: it still boggles my mind that your answer was down-voted, and that mine is the only up-vote for that answer.
@HovercraftFullOfEels I believe that the lack of upvotes on my answer is due to the fact that I missed the train. The best time for upvotes is the first few minutes of answering a question and sadly, mine had a downvote in the first few minutes. This may have mislead other's to believe that it's an incorrect answer. Thanks for your upvote that gives my answer a chance to compete.
0

Is there some way to have the classes refer to each other or do I have no choice but to transfer all of my methods into a single class?

In my opinion, cyclic references are a code-smell. See this answer for an explanation. Take special note of the point about cognitive load.

The solution is to have one class depend on the other and delegate the calls to the other class :

public class class1{
    class2 c2 = new class2(); 
    m1(){
        c2.ma(); 
        m2();
    }
    m2(){}
}

public class class2{
    ma(){}
}

This way, you are not really transferring all the methods to one class but just composing class2 into class1. Other classes that were depending on class1 and class2 only have to depend on class1 instead.

Improve this answer

1 Comment

Can the downvoter please let me know what is wrong with my answer? Does the downvoter understand Java?
0

What actually happens is that you create an instance of Class1 in the constructor,

  • which creates an instance of Class2 in the constructor,
    • which creates an instance of Class1 in the constructor,
      • which creates an instance of Class2 in the constructor,
        • et cetera.

Your constructors are recursingly creating instances, causing the call stack to flood, resulting in a StackOverflowError.

I assume you want an instance of Class1 to hold a reference to an instance of Class2 and vice versa?

In that case, you can just do this:

public class Class1 {

    private Class2 c2;

    public Class1() {
        this.c2 = new Class2(this);
    }
}

public class Class2 {

    private Class1 c1;

    public Class2(Class1 class1) {
        this.c1 = class1;
    }
}

Cyclic aggregations like this are often a sign of a bad design; but there are situations where it is perfectly valid, or at least reflects reality.

Improve this answer

2 Comments

@HovercraftFullOfEels I bet you didn't see this coming :)
StackOverflow hah!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.