4

For example, if 2 classes depend on each other, it is a kind of circular dependency and should be avoided. How about methods? for example, if I have 2 methods which call each other:

public void methodA(){
    //some other code
    if(something){
        methodB();
    }
    .
    .
    .
    //some other code
}

public void methodB(){
    //some other code
    methodA();
    //some other code
}

is it a code smell?

Note: I think In plain English, what is recursion? don't answer my question, because that question is about WHAT is recursion, while I'm asking HOW to write recursion function (and I don't know if my case is called recursion actually), i.e.: should I condense recursion code to run into a single function?

ocomfd
  • 5,652
  • 8
  • 29
  • 37
  • 1
    Possible duplicate of [In plain English, what is recursion?](https://softwareengineering.stackexchange.com/questions/25052/in-plain-english-what-is-recursion) – gnat Aug 03 '18 at 06:17
  • If you think that code smells you should have your nose checked. Don’t use this term in polite company. – gnasher729 Aug 03 '18 at 06:51
  • That `if (something)`: is that condition based on the state of the instance (hence prone to concurrency issues) or just local state? – Bernhard Hiller Aug 03 '18 at 08:02
  • Circular dependency between classes is usually bad and a "code smell". It may happen and it's not the end of the world but if you can solve it another way (e.g. keeping the two methods in one class and making them an implementation detail), it will likely be better. – meow Aug 03 '18 at 11:14
  • https://en.wikipedia.org/wiki/Mutual_recursion whether its a smell depends on the language slightly I think – jk. Aug 03 '18 at 12:11
  • 2
    @gnasher729, fyi, Wikipedia has an entry for [code smell](https://en.wikipedia.org/wiki/Code_smell), and this article seems quite innocent and polite; we even have a code-smell tag here at Programmers! While perhaps there are better terms to use, I don't think that abusing the OP [*telling them to have their nose checked*] for using this common, well-understood term is appropriate. You might try a gentler approach. – Erik Eidt Aug 03 '18 at 19:06
  • @ErikEidt "Code smell" is abusive language. Language designed to belittle others. If you use that kind of language, plenty of people will be disgusted with you. – gnasher729 Aug 03 '18 at 22:49
  • @gnasher729, Nothing you're saying justifies your abuse of the OP. – Erik Eidt Aug 03 '18 at 23:51
  • 2
    @gnasher729 I do not agree with your qualification of the term code smell, I think of it as an inconvenient pattern that is likely representative for more code in the project. How would you describe something like that in a "polite", equally concise way? – Martin Maat Aug 04 '18 at 08:21
  • I would not call it a code smell but it would be more readable if you had all your switch logic in a single construct. It belongs together. You now have to switch back and forth between methods to understand what it does, that is not good. Plus your stack will grow twice as quickly. – Martin Maat Aug 04 '18 at 08:26

3 Answers3

6

from a perspective of recursion, there is nothing wrong with having multiple functions involved in that recursive behaviour.

However, this isn't really an issue of recursion; it's an issue of coupling. Because methodA and methodB call each other, they are tightly coupled to each other. If they exist in the same class, then that's not a problem: a class is free to couple it's members to each other as it sees fit. If though, they exist in different classes, then this is potentially is an issue.

In the latter case, the standard way to get around this is pick one of them to be the "master", and to inject the other into it via a parameter or the class's constructor.

David Arno
  • 38,972
  • 9
  • 88
  • 121
2

You don't typically see this type of circular dependency, not because it is necessarily "bad code", but because generally it demonstrates a lack of understanding of relationship between two classes. When you look at it from the context of, "In order for class A to function, there must be present class B," then you can see that the opposite, "In order for class B to function, there must be present class A" doesn't make sense.

Classes can delegate tasks to other classes, but communication in the opposite direction is usually done in other ways, such as through events or through simple method returns.

Again, this isn't to say such things can't be done. The visitor pattern takes advantage of this to give control to the caller of the request. However, in all my years as a programmer, such situations don't arise often. Before establishing such a relationship, I would re-evaluate the type of task required from both classes, and pinpoint the exact logic which prevents you from establishing a parent-child relationship. In all likelihood, you're simply putting a piece of logic, which should be in the parent, into the child class.

Neil
  • 22,670
  • 45
  • 76
0

Other answers have addressed the issues of class coupling, so I won't go into that, but if we were to assume the two methods in your example are in the same class, I would still say this is (probably) bad code - I can't say for certain without more context.

The main reason it is bad is because it is very difficult to follow what the flow of execution would be just by looking at the code, and this makes the code hard to maintain.

The only reason I can think of to have two methods call each other is for recursion, and then it's important to make it simple and easy to follow - usually the call to the other method is the last thing the method does.

I made an example in ruby of a valid reason for two methods to be calling each other. It's an implementation of quicksort where the method of sorting is slightly different at each level down the tree.

Toby 1 Kenobi
  • 141
  • 4