Question

In my factory method I use Switch statement to create concrete objects. This results in very high cyclomatic complexity. Here is a sample code:

private static UnitDescriptor createUnitDescriptor(string code)
{
     switch (code)
     {
         case UnitCode.DEG_C:
             return new UnitDescriptorDegC();

         case UnitCode.DEG_F:
             return new UnitDescriptorDegF();

         :
         :
         default:
             throw new SystemException(string.format("unknown code: {o}", code);
      }
  }

How can I refactor this to reduce cyclomatic complexity? If I use reflection to create objects or something else to build objects is it better than above method?

Was it helpful?

Solution

It's possible for you to use a Dictionary to remove the switch statement entirely:

class MyClass
{
    private static Dictionary<string, Func<UnitDescriptor>> dict = new Dictionary<string, Func<UnitDescriptor>>();

    static MyClass()
    {
        dict.Add(UnitCode.DEG_C, () => new UnitDescriptorDegC());
        dict.Add(UnitCode.DEG_F, () => new UnitDescriptorDegF());
        // Other mappings...
    }

    private static UnitDescriptor createUnitDescriptor(string code)
    {
        Func<UnitDescriptor> value;
        if (dict.TryGetValue(code, out value))
        {
            return value();
        }

        throw new SystemException(string.Format("unknown code: {0}", code));
    }
}

OTHER TIPS

This question is the 1st result when googling "cyclomatic complexity factory". The use of a Dictionary greatly reduce the cyclomatic complexity however as pointed out niconoe in PHP using such approach will increase the memory usage because objects are loaded at runtime.

To solve this issue we can use Dictionary + Closures. Objects declared inside a closure are not created until the closure is called.

class NotTriggered
{

    public function __construct()
    {
        echo "not triggered\n";
    }

    public function trigger()
    {
        echo "still not triggered\n";
    }

}

class triggered
{

    public function __construct()
    {
        echo "triggered\n";
    }

    public function trigger()
    {
        echo "triggered again\n";
    }

}

function echoHello()
{
    echo "hello\n";
}

function echoWorld()
{
    echo "world\n";
}

$arr = [
    0 => fn() => echoHello(),
    1 => echoWorld(),
    2 => fn() => (new NotTriggered)->trigger(),
    3 => (new Triggered)->trigger(),
];

//output :
//world
//triggered
//triggered again

so in a factory method the code could look like this

public function createService($serviceName): ServiceInterface
{
    $services = [
        'service1' => fn() => new Service1(),
        'service2' => fn() => new Service2(),
        'service3' => fn() => new Service3(),
    ];

    return $services[$serviceName] ?? throw new \Exception('service not registered');
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top