For two methods who need to call each other: how do I prevent an infinite loop?

Question

Hello!

I have two classes (A and B) which depend on each other in following sense:
Each class has a method performing some action.
The action of each class depends on an action of the other class.

So, if the user calls action of class A, it should automatically call action of class B.
Same for the other way. But an infinite loop should be prevented.

I have found some code, which deals with this issue, but it seems to be a little dumb to me: Infinite loop is prevented by locking.

import java.util.concurrent.locks.*;
import static java.lang.System.*;
import org.junit.*;

public class TEST_DependentActions {

  static class A {
    private B b = null;
    private final ReentrantLock actionOnBLock = new ReentrantLock();

    public void setB(B b) {
      this.b = b;
    }

    public void actionOnB() {
      if (!actionOnBLock.isLocked()) {
        actionOnBLock.lock();
        b.actionOnA();
        actionOnBLock.unlock();
      }
    }
  }

  static class B {
    private A a = null;
    private final ReentrantLock actionOnALock = new ReentrantLock();

    public void setA(A a) {
      this.a = a;
    }

    public void actionOnA() {
      if (!actionOnALock.isLocked()) {
        actionOnALock.lock();
        a.actionOnB();
        actionOnALock.unlock();
      }
    }
  }

  @Test
  public void test1()
      throws Exception {

    out.println("acting on class A first:");

    A a = new A(); B b = new B();
    a.setB(b);     b.setA(a);

    a.actionOnB();
  }

  @Test
  public void test2()
      throws Exception {

    out.println("acting on class B first:");

    A a = new A(); B b = new B();
    a.setB(b);     b.setA(a);

    b.actionOnA();
  }
}

Output is as following:

acting on class A first:
A : calling class B's action.
B : calling class A's action.
acting on class B first:
B : calling class A's action.
A : calling class B's action.

Well, it works, but doesn't seem to be an optimal solution.

How would you do it?
Is there a pattern which deal with such issue?

EDIT:
I want to know it in general.
But let's say I have a Container which contains multiple Elements. The Container provides the method remove(someElement) and the Element also provides a method removeMe().
Both methods depend on each other, but can't be connected to one method, because both methods additionally perform some internal stuff, which is only accessible inside each class.

Answer 1

The solution presented looks perfectly acceptable for the given scenario. But it really depends on what the actions mean whether this behaviour is correct or not. If this is exactly the behaviour that you want, then I'm not sure you have a problem.

Addendum:
In which case I'd say, split out the 'extra stuff' into a separate method, so that remove on the container can call the 'extra stuff' on the element without the recursive call back. Similarly, the extra stuff on the container can be separated so that removeMe can call a method on the container that only does the non-recursive stuff.

Answer 2

I would handle this by rethinking the logic. Circular dependencies are typically a sign that something is a little ... off. Without more insight into the exact problem I can't be more specific.

Answer 3

My recommendation would be to think less about the code and more about the design. Can the shared operations be abstracted into a new class which both classes can communicate with? Has the functionality been incorrectly shared across 2 classes?

At Uni this year they introduce the concept of "code smells", which is a bunch of "gut reactions" that code needs re-factoring. Perhaps could help?

For an overview try: Wikipedia Code Smells or this book.

Perhaps you could tell us more about what you are trying to represent in the code?

I hope this helps.

Answer 4

Can you make one of the methods private/internal. This should ensure only the other one can be called by client code, and you always know which way the calls work.

Alternatively, using your container/element example, something like this:

public class Container
{
    public void Remove(Element e)
    {
        e.RemoveImplementation();
        RemoveImplementation();
    }

    // Not directly callable by client code, but callable
    // from Element class in the same package
    protected void RemoveImplementation()
    {
       // Mess with internals of this class here
    }
}


public class Element
{
    private Container container;

    public void Remove()
    {
       RemoveImplementation();
       container.RemoveImplementation();
    }

    // Not directly callable by client code, but callable
    // from Container class in the same package
    protected void RemoveImplementation()
    {
        // Mess with internals of this class here.
    }
}

I'm not sure if there is a common name for this pattern.

Answer 5

If an exception is raised your implementation will not get unlocked. Locks can be expensive, a slight more light weight approach is to use an AtomicBoolean

private final AtomicBoolean actionOnBLock = new AtomicBoolean();

public void actionOnB() {
    if (!actionOnBLock.getAndSet(true))
        try {
            b.actionOnA();
        } finally {
            actionOnBLock.set(false);
        }
}

As others have suggested, a better way would be to write the code so one down not have to call the other. Instead you could have a method which calls A and B so the two objects don't need to know about each other.

public class AB {
   private final A a;
   private final B b;
   public AB(A a, B b) {
     this.a = a;
     this.b = b;
   }

   public void action() {
     a.action(); // doesn't call b.
     b.action(); // doesn't call a.
   }
}

Answer 6

As others have said, I think in most situations you should try to avoid this and refactor your code so the problem does not occur. But then there a certainly situations where this might not be an option or totally unavoidable. The most general approach to this is to ensure that before method A calls method B (and vice versa), it has to ensure the object of A is in state where an additional call to A immediately returns and thus results in a no-op. This is really very abstract and can be quite hard to apply to a concrete class and implementation.

Taking your example of container and element classes (say, C and E), where the C has a method C.remove(E e) and E has a method E.remove(), you could implement it like this:

class C:
    elements = [...] // List of elements

    remove(e):
        if not elements.contains(e):
            return

        elements.remove(e)
        // Do necessary internal stuff here...
        // and finally call remove on e
        e.remove()

class E:
    container = ... // The current container of E

    remove():
        if container is none:
            return

        c = container
        container = none
        // Do necessary internal stuff here...
        // and finally call remove on c
        c.remove(this)

Answer 7

function fooA() calling fooB() and fooB() again calling fooA() is a valid recursion. Recursion does not always have to be same function calling itself.

So, how do we prevent a recursive function from infinitely looping ? By having a terminating condition right ?

I guess thats the way to go. However, I agree with other's comments on rethinking the design and avoid such recursions.

Simon's solution below also relies on the terminating conditions like

if not elements.contains(e):
            return

and

if container is none:
            return

Answer 8

i think this could be solved by a boolean value global to the class (make it an instance variable of course). it checks if([the boolean variable]) and if it is true; it runs the call to the other method, if it is false it doesn't. just inside the if statement set the check equal to false. then at the end of each method make it set to true.

thats just how i would do it though.