2

I my c# program I have to perform 5 steps(tasks) sequentially. basically these five should execute one after the other only the previous task performed is success. Currently I have done it in following style. But this is not very good code style to follow.

var isSuccess=false;
isSuccess=a.method1();

if(isSuccess)
    isSuccess=a.method2();

if(isSuccess)
    isSuccess=a.method3();

if(isSuccess)
    isSuccess=a.method4();

if(isSuccess)
    isSuccess=a.method5();

How can I re factor this code. What is the best way I can follow?

3 Answers3

7

C#/.NET process conditions left to right and would stop at the first false if they were "ANDed" together, e.g.

bool success = A() && B() && C();

should stop at B() if it returned false. Thus the following code will only output "In A" and "In B", but not "In C", because as soon as B() returns false, it stops.

using System;

public class App
{
  public static void Main()
  {
    bool success = A() && B() && C();
  }

  static bool A()
  {
    Console.WriteLine("In A");
    return true;
  }

  static bool B()
  {
    Console.WriteLine("In B");
    return false;
  }

  static bool C()
  {
    Console.WriteLine("In C");
    return true;
  }
}

Output:

In A
In B
Omer Iqbal
  • 3,264
2

I would actually let it stay as it is, since this way it's easily readable and understandeable by anyone. But if you use the same pattern in a lot of places and all the method have the same signature here's an (overengineered and probably twisted) idea:

public class SequentialExecutor
{
    private Func<bool>[] functions;

    public SequentialExecutor(params Func<bool>[] functions)
    {
        this.functions = functions;
    }

    public bool Execute()
    {
        foreach (var func in functions)
        {
            bool isSuccess = func();
            if (!isSuccess)
            {
                return false;
            }
        }

        return true;
    }
}

You can use it like this:

var executor = new SequentialExecutor(a.method1, a.method2, a.method3, a.method1, a.method4);
bool isSuccess = executor.Execute();

P.S. NOT TESTED

1

This is the sort of situation where throwing an exception on failure may be neater. If that's your API, you can then do:

try {
    a.method1();
    a.method2();
    a.method3();
    a.method4();
    a.method5();
} catch (UnsatisifedException e) {
    // Some sort of recovery logic here...
}