views:

116

answers:

4

You have a sequence of functions to execute. Case A: They do not depend on each other. Which of these is better?

function main() {
  a();
  b();
  c();
}

or

function main() {
  a();
}

function a() {
  ...
  b();
}

function b() {
  ...
  c();
}

Case B: They do depend on successful completion of the previous.

function main() {
  if (a())
    if (b())
      c();
}

or

function main() {
  if (!a()) return false;
  if (!b()) return false;
  c();
}

or

function main() {
  a();
}

function a() {
  ... // maybe return false
  b();
}

funtion b() {
  ... // maybe return false
  c();
}

Better, of course, means more maintainable and easier to follow.

+13  A: 

Case A: 1.
Reasoning: Since none of the functions depend on each other, calling them in order in main shows a logical sequence of events. Your alternative where they call the next one at the end of each function just looks like spaghetti code, and is hard for someone reading your program to follow.

Case B: None of the above.

function main() {
    return a() && b() && c();
}

Reasoning: It seems you don't really care about the return value, you just want to break out early if one of the functions returns a certain value. You can return the "anded" result of all of these functions, and if any one of these returns false the code will break out early. So, if a returns false then b wont be executed. Placing all of them on one line is easy to read and concisely shows that they are dependent on each other.

Corey Sunwold
+1 for elegance
Manos Dilaverakis
A: 

Case A: first option

Case B: second option

Paul R
+2  A: 

Case A: first option

If you use the second option, you make it much more difficult to re-use a because you pull in b and c automatically.

Case b: depends - do a, b, and c return boolean values naturally, or some other value to check? I still wouldn't have a call b and b call c, because then you introduce an unnecessary dependency. If return values make sense, I lean towards option 2 - less nesting is a good thing.

justkt
+1  A: 

First off, the best answer will depend on multiple aspects on the context the code is in - there's no one right answer other than 'it depends'.

However, taking at face value, Case A:

Option 1 shows a top level view of the algorithm.

Option 2 hides this, the calls to B & C are hidden. It could be quite a lot of work to discover the C was called. Also, it's harder to test A & B in isolation if they then call further methods. Main can always be tested if A,B and C can be mocked.

Case B: It's common to have arguments over having 1 exit point vs multiple exit points in a method. Sometimes multiple exit points can make code simpler, which makes it more maintainable, other times it's not obvious what impact a return has, or that a return will occur, which makes it less maintainable.

Option 1 shows the algorithm explicitly, but multiple nested if's can quickly get out of hand. In this sort of case, I'd definitely have option 2 or some clever boolean variant (yes, corey).

Option 3 looks simply obtuse to me.

Jim T