How to streamline class initialization within an object based on a parameter type-CodePudding

Based on a passed parameter containing a type I'm looking for a better way to initialize the class based on that type. Currently I'm "solving" it with a switch statement which is feels quite redundant.

Bad example showing what the functionality is:

protected Quest GenerateQuest(QuestConfiguration questConfiguration)
{

    List<QuestObjective> objectives = new();
    foreach ( QuestObjectiveConfiguration questConfigurationObjective in questConfiguration.ObjectiveConfigurations)
    {
        switch (questConfigurationObjective.ObjectiveType)
        {
            case ObjectiveType.Fetch:
                objectives.Add(new ObjectiveFetch(questConfigurationObjective.InternalId));
                break;
            case ObjectiveType.Gather:
                objectives.Add(new ObjectiveGather(questConfigurationObjective.InternalId));
                break;
            case ObjectiveType.Craft:
                objectives.Add(new ObjectiveCraft(questConfigurationObjective.InternalId));
                break;
            case ObjectiveType.Deliver:
                objectives.Add(new ObjectiveDeliver(questConfigurationObjective.InternalId));
                break;
            case ObjectiveType.Combat:
                objectives.Add(new ObjectiveCombatEncounter(questConfigurationObjective.InternalId));
                break;
            default:
                throw new ArgumentOutOfRangeException();
        }
    }
    return new Quest(objectives);
}

Looked/googled for suiting patterns but I'm having a hard time getting specific results because the related terms are generic and the issue is an edgecase.

CodePudding user response：

With Linq and switch expression you can streamline your method's implementation like this:

protected Quest GenerateQuest(QuestConfiguration questConfiguration)
{
    List<QuestObjective> objectives = questConfiguration.ObjectiveConfigurations
    .Select(questConfigurationObjective =>
    {
        var id = questConfigurationObjective.InternalId;
        QuestObjective objective = questConfigurationObjective.ObjectiveType switch
        {
            ObjectiveType.Fetch => new ObjectiveFetch(id),
            ObjectiveType.Gather => new ObjectiveGather(id),
            ObjectiveType.Craft => new ObjectiveCraft(id),
            ObjectiveType.Deliver => new ObjectiveDeliver(id),
            ObjectiveType.Combat => new ObjectiveCombatEncounter(id),
            _ => throw new ArgumentOutOfRangeException()
        };
        return objective;
    })
    .ToList();
    
    return new Quest(objectives);
}

or you can make it even more concise like this

protected Quest GenerateQuest(QuestConfiguration questConfiguration)
    => new Quest(questConfiguration.ObjectiveConfigurations
    .Select(questConfigurationObjective =>
    {
        var id = questConfigurationObjective.InternalId;
        return questConfigurationObjective.ObjectiveType switch
        {
            ObjectiveType.Fetch => new ObjectiveFetch(id),
            ObjectiveType.Gather => new ObjectiveGather(id),
            ObjectiveType.Craft => new ObjectiveCraft(id),
            ObjectiveType.Deliver => new ObjectiveDeliver(id),
            ObjectiveType.Combat => new ObjectiveCombatEncounter(id),
            _ => throw new ArgumentOutOfRangeException()
        };
    })
    .ToList());

CodePudding user response：

If the QuestObjective can be created with just QuestObjectiveConfiguration, then I'd consider having the switch statement in a method or extension method of it.

This of course does not get rid of the switch statement but atleast keeps it in one place.

static class QuestObjectiveConfigurationExtensions
{
    public static QuestObjective ToQuestObjective(
        this QuestObjectiveConfiguration config) => config.ObjectiveType switch
    {
        ObjectiveType.Fetch => new ObjectiveFetch(config.InternalId),
        //rest of your types
        _ => //maybe throw an Exception here
    };
}

This shortens your objective creation to

var objectives = questConfiguration.ObjectiveConfigurations.Select(x => x.ToQuestObjective)

You can do the same for QuestConfiguration and Quest of course and implement a ToQuest method.

One option to get rid of the switch statement completely would be to have different types of QuestObjectiveConfiguration that implement different methods of ToQuestObjective.

CodePudding user response：

In case your framework version doesn't support switch expression that Peter Csala suggesed, I have a more traditional approach like:

    Quest GenerateQuest(QuestConfiguration questConfiguration)
    {

        List<QuestObjective> objectives = new();
        Dictionary<ObjectiveType, Type> lstActions = new Dictionary<ObjectiveType, Type>();
        lstActions.Add(ObjectiveType.Fetch, typeof(ObjectiveFetch));
        lstActions.Add(ObjectiveType.Gather, typeof(ObjectiveGather));
        lstActions.Add(ObjectiveType.Craft, typeof(ObjectiveCraft));
        lstActions.Add(ObjectiveType.Deliver, typeof(ObjectiveDeliver));
        lstActions.Add(ObjectiveType.Combat, typeof(ObjectiveCombatEncounter));

        foreach (QuestObjectiveConfiguration questConfigurationObjective in questConfiguration.ObjectiveConfigurations)
        {
            if (lstActions.ContainsKey(questConfigurationObjective.ObjectiveType))
                objectives.Add((QuestObjective)Activator.CreateInstance(lstActions[questConfigurationObjective.ObjectiveType], new[] { questConfigurationObjective.InternalId }));
            else
                throw new ArgumentOutOfRangeException();
            }
        }
        return new Quest(objectives);
    }