Who should be responsible for calling a method?

Question

If I have complex a task to solve I sometimes end up in a situation where I have one method that controls the execution. Because of null checks, if statements, calling methods that map between types and so on, this method can become really long and I struggle to make it simpler.

Example 1

public class A
public string MethodA(string stringA)
{
    var fooResult = _fooService.fooMethod(stringA);

    if(fooResult)
    var barResult = _barService.barMethod(fooResult);

    if(barResult)
    // And so on..

    return someResult;
}

I could chain the method calls which makes the first method simpler. But this makes the fooMethod dependant on the _barService, and the barMethod dependant on the _someService and so on.

Example 2 (same as above but with chained method calls)

public class B
public string MethodB(string stringB)
{
    return _fooService.fooMethod(stringB);
}

public class Foo
public string fooMethod(string stringB)
{
    return _barService.barMethod(stringB);
}

public class Bar
public string barMethod(string fooString)
{
    return _someService.someMethod(fooString);
    // And so on...
}

How should I structure my code? And how should I think when it comes to which method that is responsible for calling another method?

In other words, should I do like this:

class A
{
  Execute()
  {
    A();
    B();
    C();
  }

  method A()
  {
  ...
  }

  method B()
  {
  ...
  }

  method C()
  {
  ...
  }
}

Or like this:

class B
{
  Execute()
  {
    A();
  }

  method A()
  {
    B();
  }

  method B()
  {
    C();
  }

  method C()
  {
  ...
  }
}

Answer 1

It depends.

If the stages are well defined calling them in order will be more convenient in most cases.

However if for example B is well defined but A and C are really just the start and end of a job that depends on B, such that neither A or C make much sense on their own and are hard to name/describe or to assess whether they have been successful, then in that case combining A and C in a single method that calls B makes more sense. Separating the methods so that each does a specific task is good, but you should avoid separating things to the point that they only do part of a task if that partial task doesn't have much meaning on its own.

You should consider how you will test the code - is it easier for you with one design over the other.

Consider how you would maintain the code. This is where ease of understanding the code matters most, but also consider if one stage needs editing to fix a bug or to enhance functionality; which design will allow you to do this with the minimum of effort, and side effects on other code and the associated re-testing of areas that shouldn't really have been affected?

Think whether you will ever need to vary one of the steps in the future - to call alternative versions of B depending on some input or setting?

Consider reusability. If A calls B you can't reuse A in a situation where you didn't want to also call B, in your other design you could reuse A.

Will they always need calling in that order - will there be scope for optimising speed by calling B and C in parallel ( calling C from B doesn't allow that ).

If B fails should you still try to call C ( I suspect most people would say not because that's more likely to true, but it really does depend on what the real problem is) separate calls usually makes that easier than chaining.

In favour of calling B from A, you'd only need to expose A in the interface and (if your chosen language allows it) you could make B and C private. Its then much easier to ensure that B is only ever called from A and so B may be able to avoid testing certain initial conditions are true and just rely on A having done its job.

Of course the above is more a set of questions than answers and that's because either design might be right depending on what your real-life task is, but hopefully the above considerations help you decide.

Answer 2

There is no universal decision for all your tasks. Basically you should strive to write your methods short and implementing one task. Proper methods naming will help you do that (seems like this is your problem). For example avoid names like ExampleClass.DoWork() or ExampleClass.ManageObject(object) as they not exact and induce complex and long implementation.

Generally, "serial" version of your last two examples more preferable as it easier to read, and A(), B(), C() likely to have more trivial implementation, and it would be easier to name they properly.

For more specific advices you can post your code in Code Review.