-1
\$\begingroup\$

I'm working on a segment of code where it runs a number of tasks and then combine individual task results to construct a complete task result object, there's no concurrency involved so it's purely a question of the best use of class inheritance/pattern.

Below is the skeleton of the code I've written, it loops through IList<ITask> where each ITask returns an ITaskResult, the main method will then have to examine the type of each ITaskResult and set the property on CompleteTaskResult accordingly.

The goal is to keep the code easy to understand and easy to add new Task/TaskResult.

What I'm not happy about is the part where it has to check the type of ITaskResult individually and then set the value on CompleteTaskResult, it feels a little cumbersome with lots of repetitions.

Is there a better way to structure this code?

Thanks.

        public interface ITask
        {
            ITaskResult Execute();
        }

        public class AlphaTask : ITask
        {
            public ITaskResult Execute()
            {
                return new AlphaTaskResult();
            }
        }

        public class BetaTask : ITask
        {
            public ITaskResult Execute()
            {
                return new BetaTaskResult();
            }
        }

        public class GammaTask : ITask
        {
            public ITaskResult Execute()
            {
                return new GammaTaskResult();
            }
        }

        public interface ITaskResult
        {
        }

        public class AlphaTaskResult : ITaskResult
        {
        }

        public class BetaTaskResult : ITaskResult
        {
        }

        public class GammaTaskResult : ITaskResult
        {
        }

        public class CompleteTaskResult
        {
            public AlphaTaskResult AlphaTaskResult { get; set; }
            public BetaTaskResult BetaTaskResult { get; set; }
            public GammaTaskResult GammaTaskResult { get; set; }
        }

        static void Main(IList<ITask> tasks)
        {
            var taskResults = tasks.Select(x => x.Execute());

            var completeTaskResult = new CompleteTaskResult();

            foreach (var taskResult in taskResults)
            {
                if (taskResult is AlphaTaskResult alphaTaskResult)
                {
                    completeTaskResult.AlphaTaskResult = alphaTaskResult;
                    continue;
                }

                if (taskResult is BetaTaskResult betaTaskResult)
                {
                    completeTaskResult.BetaTaskResult = betaTaskResult;
                    continue;
                }

                if (taskResult is GammaTaskResult gammaTaskResult)
                {
                    completeTaskResult.GammaTaskResult = gammaTaskResult;
                    continue;
                }
                throw new InvalidOperationException("unsupported task")
            }

        }

Edit: The intention is one can pass a number of tasks in the form of IList<ITask> to the library and then retrieve concrete results back from CompleteTaskResult CompleteTaskResult may have null properties if no specific tasks related to the properties were executed. The main caller is well aware of what concrete type of ITask were sent in so it also knows what properties from CompleteTaskResult to query for the results

\$\endgroup\$

closed as off-topic by πάντα ῥεῖ, Peter Taylor, 200_success, dfhwze, pacmaninbw Jul 18 at 16:00

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – πάντα ῥεῖ, Peter Taylor, 200_success, dfhwze, pacmaninbw
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ Why does CompleteTaskResult contain specific result types, instead of a list of ITaskResults? If that is intentional, then why does Main accept a list of ITasks, instead of specific task types whose output match the properties in CompleteTaskResult? (Note that your code is fairly abstract. That tends to result in downvotes due to lack of concrete context). \$\endgroup\$ – Pieter Witvoet Jul 18 at 13:14
  • 2
    \$\begingroup\$ Your question lacks concrete context about your specific design desicions. Why do you need to know the specific type of ITaskResult at all. Give it a meaningful interface, with properties and functions you can use independently of the underlying type. Note that asking for reviewing stub code like that is off-topic here. – πάντα ῥεῖ 8 mins ago \$\endgroup\$ – πάντα ῥεῖ Jul 18 at 13:26
  • 4
    \$\begingroup\$ This code is too sketchy and hypothetical to review. Foo/bar examples are not acceptable on Code Review. What do these tasks have in common, and how do they differ? You need to post your real code, or at least something plausibly realistic, so that we can give you proper advice. Please see How to Ask. \$\endgroup\$ – 200_success Jul 18 at 13:52
  • 2
    \$\begingroup\$ @πάνταῥεῖ thanks, I'll ask for advise again after I get it fully working, my apologies. \$\endgroup\$ – Godsent Jul 18 at 13:57
  • 2
    \$\begingroup\$ @Godsent: that makes little sense. If your method accepts a list of ITask, then it should return a list of ITaskResult (if necessary, a caller could use Linq's OfType<T> to fetch specific results). If it specifically returns only an alpha, beta and gamma result, then it should accept only an alpha, beta and gamma task. Otherwise, what about callers that pass in omega tasks? Or multiple alpha tasks? \$\endgroup\$ – Pieter Witvoet Jul 18 at 14:06
1
\$\begingroup\$

You could simply add the task results to a collection. Different types of collections can be used:

  1. List<ITaskResult>: You can add several results of the same type. You must enumerate to find a task of a specific type.

  2. Dictionary<Type, ITaskResult>: Each task result type can be added only once. You can query specific task types.

    if (results.TryGetValue(typeof(BetaTaskResult), out ITaskResult taskResult)) {
        ...
    }
    

    or if you know a result is there for sure:

    ITaskResult taskResult = results[typeof(BetaTaskResult)];
    

    Of course, you can also use other types of keys, like enums or strings.

  3. Dictionary<Type, List<ITaskResult>>: Each task result type can be added several times. You can query specific task result types. Handling is a bit more complex.

If you want to access members who are not part of the interface, you must cast the result to specific types.

// Assuming each result type occurs only once.
Dictionary<Type, ITaskResult> results = tasks
    .Select(x => x.Execute())
    .ToDictionary(r => r.GetType()); // alternative: .ToList()

If you prefer to keep your current solution, you can simplify it a bit. This will enumerate the tasks several times; however, this is acceptable, because you have a very small number of tasks.

var taskResults = tasks
    .Select(x => x.Execute())
    .ToList();
var completeTaskResult = new CompleteTaskResult {
   AlphaTaskResult = taskResults.OfType<AlphaTaskResult>().FirstOrDefault(),
   BetaTaskResult  = taskResults.OfType<BetaTaskResult>().FirstOrDefault(),
   GammaTaskResult = taskResults.OfType<GammaTaskResult>().FirstOrDefault(),
};

It is important to call .ToList(), otherwise this would execute the tasks several times.

See also: Lazy Evaluation (and in contrast, Eager Evaluation)

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.