samedi 22 juillet 2023

Breaking up the rules to call a strategy from the strategy algorithm

So let's say I have this class


var intItems = new List<int> { 200, 2, 3 };
var stringItems = new List<string> { "Hello", " ", "World!" };
var intItems2 = new List<int> { 10, 2, 3 };

var consolidator = new Consolidator();
var s = consolidator.ConsolidateToString(intItems);
Console.WriteLine(s);

s = consolidator.ConsolidateToString(stringItems);
Console.WriteLine(s);

s = consolidator.ConsolidateToString(intItems2);
Console.WriteLine(s);


public class Consolidator
{
    public string ConsolidateToString<T>(IEnumerable<T> items)
    {
        ArgumentNullException.ThrowIfNull(items);
        string consolidatedString = string.Empty;

        if (items is IEnumerable<int> subtract && subtract.Any(i => i > 100))
        {
            consolidatedString = subtract.Aggregate((total, value) => total - value).ToString();
        }
        else if (items is IEnumerable<int> intItems)
        {
            consolidatedString = intItems.Aggregate((total, value) => total + value).ToString();
        }
        else if (items is IEnumerable<string>)
        {
            consolidatedString = string.Concat(items);
        }
        else
        {
            throw new ArgumentException("No valid consolidation found.");
        }

        return consolidatedString;
    }
}

I get the results

195
Hello World!
15

This design works but has issues. For one, hard to test and it can become pretty complex with many if conditions. It will also become a maintenance hell once you start adding pieces.

So strategy 2

var intItems = new List<int> { 200, 2, 3 };
var intItems2 = new List<int> { 10, 2, 3 };
var stringItems = new List<string> { "Hello", " ", "World!" };
var consolidationStrategies = new List<IConsolidator> { new SubtractIntConsolidator(),  new IntConsolidator(), new StringConsolidator() };

var consolidatorStrategy = consolidationStrategies.FirstOrDefault(cs => cs.CanHanldeType(intItems));
if (consolidatorStrategy is null)
{
    throw new Exception("No valid consolidation found.");
}

var s = consolidatorStrategy.ConsolidateToString(intItems);
Console.WriteLine(s);

consolidatorStrategy = consolidationStrategies.FirstOrDefault(cs => cs.CanHanldeType(stringItems));
if (consolidatorStrategy is null)
{
    throw new Exception("No valid consolidation found.");
}

s = consolidatorStrategy.ConsolidateToString(stringItems);
Console.WriteLine(s);

consolidatorStrategy = consolidationStrategies.FirstOrDefault(cs => cs.CanHanldeType(intItems2));
if (consolidatorStrategy is null)
{
    throw new Exception("No valid consolidation found.");
}

s = consolidatorStrategy.ConsolidateToString(intItems2);
Console.WriteLine(s);


public interface IConsolidator
{
    bool CanHanldeType<T>(IEnumerable<T> items);

    string ConsolidateToString<T>(IEnumerable<T> items);
}

public class IntConsolidator : IConsolidator
{
    public bool CanHanldeType<T>(IEnumerable<T> items)
    {
        return items is IEnumerable<int>;
    }

    public string ConsolidateToString<T>(IEnumerable<T> items)
    {
        var intItems = items as IEnumerable<int>;
        return intItems.Aggregate((total, value) => total + value).ToString();
    }
}

public class StringConsolidator : IConsolidator
{
    public bool CanHanldeType<T>(IEnumerable<T> items)
    {
        return items is IEnumerable<string>;
    }

    public string ConsolidateToString<T>(IEnumerable<T> items)
    {
        var intItems = items as IEnumerable<string>;
        return string.Concat(items);
    }
}

public class SubtractIntConsolidator : IConsolidator
{
    public bool CanHanldeType<T>(IEnumerable<T> items)
    {
        return items is IEnumerable<int> intTypes && intTypes.Any(i => i > 100);
    }

    public string ConsolidateToString<T>(IEnumerable<T> items)
    {
        var intItems = items as IEnumerable<int>;
        return intItems.Aggregate((total, value) => total - value).ToString();
    }
}

The results

195
Hello World!
15

Awesome, it works and has a much better design.

The next day another person runs the program and gets

205
Hello World!
15

Oh no, they switched the arrays from

var consolidationStrategies = new List<IConsolidator> { new SubtractIntConsolidator(), new IntConsolidator(), new StringConsolidator() };

to

var consolidationStrategies = new List<IConsolidator> { new IntConsolidator(), new SubtractIntConsolidator(), new StringConsolidator() };

This seems to be a really fragile design. The other issue is the logic to call the strategy is hidden inside the strategy itself. So anyone creating a new strategy would need to look at each strategy to see if their strategy overlaps with any current ones. If it does, you would need to have the arrays in the correct order. I think this makes it a bit hard to troubleshoot and leaves too much room for error.

So strategy 3

var intItems = new List<int> { 200, 2, 3 };
var intItems2 = new List<int> { 10, 2, 3 };
var stringItems = new List<string> { "Hello", " ", "World!" };

var consolidatorFactory = new ConsolidatorFactory();
var consolidatorStrategy = consolidatorFactory.GetConsolidator(intItems);

var s = consolidatorStrategy.ConsolidateToString(intItems);
Console.WriteLine(s);

consolidatorStrategy = consolidatorFactory.GetConsolidator(stringItems);
s = consolidatorStrategy.ConsolidateToString(stringItems);
Console.WriteLine(s);

consolidatorStrategy = consolidatorFactory.GetConsolidator(intItems2);
s = consolidatorStrategy.ConsolidateToString(intItems2);
Console.WriteLine(s);

public class ConsolidatorFactory
{
    public IConsolidator GetConsolidator<T>(IEnumerable<T> items)
    {
        IConsolidator consolidator = null;
        if (items is IEnumerable<string>)
        {
            consolidator = new StringConsolidator();
        }
        else if (items is IEnumerable<int> intTypes && intTypes.Any(i => i > 100))
        {
            consolidator = new SubtractIntConsolidator();
        }
        else if (items is IEnumerable<int>)
        {
            consolidator = new IntConsolidator();
        }
        else
        {
            throw new ArgumentException("No valid consolidation found.");
        }

        return consolidator;
    }
}

public interface IConsolidator
{
    string ConsolidateToString<T>(IEnumerable<T> items);
}

public class IntConsolidator : IConsolidator
{
    public string ConsolidateToString<T>(IEnumerable<T> items)
    {
        var intItems = items as IEnumerable<int>;
        return intItems.Aggregate((total, value) => total + value).ToString();
    }
}

public class StringConsolidator : IConsolidator
{
    public string ConsolidateToString<T>(IEnumerable<T> items)
    {
        var intItems = items as IEnumerable<string>;
        return string.Concat(items);
    }
}

public class SubtractIntConsolidator : IConsolidator
{
    public string ConsolidateToString<T>(IEnumerable<T> items)
    {
        var intItems = items as IEnumerable<int>;
        return intItems.Aggregate((total, value) => total - value).ToString();
    }
}

The results

195
Hello World!
15

Awesome, it works. However, we're back to a class with a complex if condition, which is not great when it comes to maintainability.

So the question is, is there a better approach?

Thanks, Michael

Aucun commentaire:

Enregistrer un commentaire