How to solve circular dependency?

Question

I have three classes that are circular dependant to each other:

TestExecuter execute requests of TestScenario and save a report file using ReportGenerator class. So:

• TestExecuter depends on ReportGenerator to generate the report
• ReportGenerator depends on TestScenario and on parameters set from TestExecuter.
• TestScenario depends on TestExecuter.

Can't figure out how to remove thoses dependencies.

public class TestExecuter {

  ReportGenerator reportGenerator;  

  public void getReportGenerator() {
     reportGenerator = ReportGenerator.getInstance();
     reportGenerator.setParams(this.params);
     /* this.params several parameters from TestExecuter class example this.owner */
  }

  public void setTestScenario (TestScenario  ts) {
     reportGenerator.setTestScenario(ts); 
  }

  public void saveReport() {
     reportGenerator.saveReport();    
  }

  public void executeRequest() {
    /* do things */
  }
}

public class ReportGenerator{
    public static ReportGenerator getInstance(){}
    public void setParams(String params){}
    public void setTestScenario (TestScenario ts){}
    public void saveReport(){}
}

public class TestScenario {

    TestExecuter testExecuter;

    public TestScenario(TestExecuter te) {
        this.testExecuter=te;
    }

    public void execute() {
        testExecuter.executeRequest();
    }
}

public class Main {
    public static void main(String [] args) {
      TestExecuter te = new TestExecuter();
      TestScenario ts = new TestScenario(te);

      ts.execute();
      te.getReportGenerator();
      te.setTestScenario(ts);
      te.saveReport()
    }
}

EDIT: in response to an answer, more details about my TestScenario class:

public class TestScenario {
    private LinkedList<Test> testList;
    TestExecuter testExecuter;

    public TestScenario(TestExecuter te) {
        this.testExecuter=te;
    }

    public void execute() {
        for (Test test: testList) {
            testExecuter.executeRequest(test); 
        }
    }
}

public class Test {
  private String testName;
  private String testResult;
}

public class ReportData {
/*shall have all information of the TestScenario including the list of Test */
    }

An example of the xml file to be generated in case of a scenario containing two tests:

<testScenario name="scenario1">
   <test name="test1">
     <result>false</result>
   </test>
   <test name="test1">
     <result>true</result>
   </test>
</testScenario >

Answer 1

Technically, you can resolve any cyclic dependency by using interfaces, as shown in the other answers. However, I recommend to rethink your design. I think it is not unlikely you can avoid the need for additional interfaces completely, while your design becomes even simpler.

I guess it is not necessary for a ReportGenerator to depend on a TestScenario directly. TestScenario seems to have two responsibilites: it is used for test execution, and it works also as a container for the results. This is a violation of the SRP. Interestingly, by resolving that violation, you will get rid of the cyclic dependency as well.

So instead of letting the report generator grab data from the test scenario, pass the data explicitly by using some value object. That means, replace

   reportGenerator.setTestScenario(ts); 

by some code like

reportGenerator.insertDataToDisplay(ts.getReportData()); 

The method getReportData needs to have a return type like ReportData, a value object which works as a container for the data to be displayed in the report. insertDataToDisplay is a method which expects an object of exactly that type.

This way, ReportGenerator and TestScenario will both depend on ReportData, which depends on nothing else, and the first two classes do not depend on each other any more.

As a second approach: to resolve the SRP violation let TestScenario be responsible for holding the results of a test execution, but not for calling the test executer. Consider to reorganize the code so not the test scenario accesses the test executer, but the test executer is started from outside and writes the results back into the TestScenario object. In the example you showed us, that will be possible by making the access to LinkedList<Test> inside of TestScenario public, and by moving the execute method from TestScenario to somewhere else, maybe directly into a TestExecuter, maybe into a new class TestScenarioExecuter.

That way, TestExecuter will depend on TestScenario and ReportGenerator, ReportGenerator will depend on TestScenario, too , but TestScenario will depend on nothing else.

And finally, a third approach: TestExecuter has too many responsibilities, too. It is responsible for executing tests as well as for providing a TestScenario to a ReportGenerator. Put these two responsibilities into two separate classes, and your cyclic dependency will vanish again.

There may be more variants to approach your problem, but I hope you get the general idea: your core problem are classes with too many responsibilities. Solve that problem, and you will get rid of the cyclic dependency automatically.

Answer 2

By using interfaces you can solve the circular dependency.

Current design:

Proposed design:

In the proposed design concrete classes don't depend on other concrete classes but only on abstractions (interfaces).

Important:

You have to use the creational pattern of your choice (maybe a factory) to avoid perfoming new of any concrete classes inside any other concrete class or calling getInstance(). Only the factory will have dependencies on concrete classes. Your Main class could serve as factory if you think a dedicated factory would be overkill. For example you can inject a ReportGenerator into TestExecuter instead of calling getInstance() or new.

Answer 3

Since TestExecutor only uses ReportGenerator internally, you should be able to define an interface for it, and refer to the interface in TestScenario. Then TestExecutor depends on ReportGenerator, ReportGenerator depends on TestScenario, and TestScenario depends on ITestExecutor, which doesn't depend on anything.

Ideally you'd be defining interfaces for all your classes and expressing dependencies through them, but this is the smallest change which will solve your problem.