Is this usage of the dynamic keyword an indicator of a design fault?

Question

I feel like I'm likely violating a design principal or two here so I'd appreciate any mistakes or antipatterns being named and pointed out.

I'm attempting to map a system in code which represents a decision tree, so based on the users input would provide them the next choice they can make, then from their input to that the next, and so on.

The response to each choice is either:

• Binary Yes/No, so two different possible follow up choices.
• 1-10 range, so multiple possible follow up choices.

Any choice type can return any other choice type so I've been aiming to have the result for the follow up choice to always be a generic IChoice.

BinaryChoice current = NotifyPersonOrNot();
IChoice next = current.MakeChoice(true); // Binary choice
IChoice next1 = next.MakeChoice(5); // Range choice
IChoice next2 = next1.MakeChoice(3); // Range Choice

An issue I see with my current solution is usage of the dynamic keyword in my IChoice MakeChoice(dynamic answer) function. I used this to avoid having to do a specific cast for the T argument in the Func<T, IChoice> GetNextChoice function that the MakeChoice function calls with it's answer input.

I considered adding a

So the IChoice interface is like so as this is the only function I want to be used.

public interface IChoice
{
    // I feel like having to use dynamic here is a sign of a mistake in the design
    public IChoice MakeChoice(dynamic answer);
}

My base class looks like this

public abstract class Choice<T> : IChoice
{
    public Choice(Func<T, IChoice> getNextChoice)
    {
        GetNextChoice = getNextChoice;
    }

    public Func<T, IChoice> GetNextChoice { get; set; }

    // This seems cleaner than type checking but I'm thinking having to use dynamic indicates a mistake in class design
    public IChoice MakeChoice(dynamic answer)
    {
        return GetNextChoice(answer);
    }
}

And an implementation of the base class is like so, where Choice is given a Type. The RangeChoice version implements Choice<int>

public class BinaryChoice : Choice<bool>
{
    public override ChoiceType ChoiceType => ChoiceType.Binary;

    public BinaryChoice(Func<bool, IChoice> getNextChoice) : base(getNextChoice) { }
}

The idea is that GetNextChoice can be set with the unique logic required to provide correct next choice in the tree.

 private BinaryChoice NotifyPersonOrNot()
    {
        // If we were returning a RangeChoice here we'd have more conditions in the function
        return new BinaryChoice((sendNotification) =>
        {
            if (!sentNotification)
            {
                // This is another BinaryChoice
                return AskAnotherYesNoQuestion();
            }

            // This is a RangeChoice
            return AskRatingQuestion();
        });
    }

I've considered changing the GetNextChoice delegate to Func<object, IChoice> but I would like to avoid the overhead of having to cast from object into the required type when defining each choice. It would preferable to do it once in a single place if it must always happen.

I've looked at adding a abstract ChoiceType enum to the Choice class and handling the type casting in the MakeChoice function but I'm not sure how to do this without explicitly setting a type for the GetNextChoice delegate.

Answer 1

I believe your question is more aimed at a what, where, and when to use the dynamic keyword more than a design philosophy. It just so happens that a good explanation exists here: https://cloudncode.blog/2016/07/12/c-dynamic-keyword-what-why-when/

The short and easy from the link:

1. There is no compile-time type checking, this means that unless you have 100% confidence in your unit tests (cough) you are running a risk.
2. The dynamic keyword uses more CPU cycles than your old fashioned statically typed code due to the additional runtime overhead, if performance is important to your project (it normally is) don’t use dynamic.
3. Common mistakes include returning anonymous types wrapped in the dynamic keyword in public methods. Anonymous types are specific to an assembly, returning them across assembly (via the public methods) will throw an error, even though simple testing will catch this, you now have a public method which you can use only from specific places and that’s just bad design.
4. It’s a slippery slope, inexperienced developers itching to write something new and trying their best to avoid more classes (this is not necessarily limited to the inexperienced) will start using dynamic more and more if they see it in code, usually I would do a code analysis check for dynamic / add it in code review.

In my personal opinion - These functions are accepting rather simple data types (bool, int). It's more cumbersome on the CPU/Hardware to have a dynamic used than to just use function overloading to accomplish the same task without the needed stress. It is true that this may eliminate the need of type checking but you run a very large risk if these functions are used in uncontrolled events.

Also, your confidence in the data being entered for MakeChoice() or any of the other dynamic parameter functions or functions using dynamic has to be as accurate as the sun coming up tomorrow. Otherwise, you may get runtime errors.

Let me know if this helps - thanks.