user10860402
user10860402

Reputation: 1012

Botframework v4: How to simplify this waterfall dialog?

I have this code but but i think its over-complicated and can be simplified. Also is there a way to go back to a spefici waterfall step if ever the user type "back" without restarting the whole dialog? I am new to this and it's hard to find a guide or online course on botframework v4 since it is new. Any help would be appreciated thanks!

  public GetNameAndAgeDialog(string dialogId, IEnumerable<WaterfallStep> steps = null) : base(dialogId, steps)
    {
        var name = "";
        var age = "";

        AddStep(async (stepContext, cancellationToken) =>
        {
            return await stepContext.PromptAsync("textPrompt",
                new PromptOptions
                {
                    Prompt = stepContext.Context.Activity.CreateReply("What's your name?")
                });
        });

        AddStep(async (stepContext, cancellationToken) =>
        {
            name = stepContext.Result.ToString();

            return await stepContext.PromptAsync("numberPrompt",
                new PromptOptions
                {
                    Prompt = stepContext.Context.Activity.CreateReply($"Hi {name}, How old are you ?")
                });
        });

        AddStep(async (stepContext, cancellationToken) =>
        {
            age= stepContext.Result.ToString();

            return await stepContext.PromptAsync("confirmPrompt",
              new PromptOptions
              {
                  Prompt = stepContext.Context.Activity.CreateReply($"Got it you're {name}, age {age}. {Environment.NewLine}Is this correct?"),
                  Choices = new[] {new Choice {Value = "Yes"},
                                   new Choice {Value = "No"},
                   }.ToList()
              });

        });

        AddStep(async (stepContext, cancellationToken) =>
        {
            var result = (stepContext.Result as FoundChoice).Value;

            if(result == "Yes" || result == "yes" || result == "Yeah" || result == "Correct" || result == "correct")
            {
                var state = await (stepContext.Context.TurnState["FPBotAccessors"] as FPBotAccessors).FPBotStateAccessor.GetAsync(stepContext.Context);
                state.Name = name;
                state.Age = int.Parse(age);

                return await stepContext.BeginDialogAsync(MainDialog.Id, cancellationToken);
            }
            else
            {
                //restart the dialog
                return await stepContext.ReplaceDialogAsync(GetNameAndAgeDialog.Id);
            }

        });

    }

    public static string Id => "GetNameAndAgeDialog";
    public static GetNameAndAgeDialog Instance { get; } = new GetNameAndAgeDialog(Id);
}

And this is my accessors code:

    public class FPBotAccessors
{
    public FPBotAccessors(ConversationState conversationState)
    {
        ConversationState = conversationState ?? throw new ArgumentNullException(nameof(conversationState));
    }

    public static string FPBotAccessorName { get; } = $"{nameof(FPBotAccessors)}.FPBotState";
    public IStatePropertyAccessor<FPBotState> FPBotStateAccessor { get; internal set; }

    public static string DialogStateAccessorName { get; } = $"{nameof(FPBotAccessors)}.DialogState";
    public IStatePropertyAccessor<DialogState> DialogStateAccessor { get; internal set; }
    public ConversationState ConversationState { get; }
    //
    public static string ConversationFlowName { get; } = "ConversationFlow";
    public IStatePropertyAccessor<ConversationFlow> ConversationFlowAccessor { get; set; }
}

Upvotes: 3

Views: 1701

Answers (1)

Drew Marsh
Drew Marsh

Reputation: 33379

So, there are a couple issues with your code and things you can do to make it better.

State within the dialog

First, let's start with the fact that you're closing over local variables in your constructor and accessing those from the closures that represent your steps. This "works" right now but is ultimately flawed. The initial flaw is different depending on the approach you've taking with instancing your GetNameAndAgeDialog dialog.

If you're using it as a singleton, that means all active conversations between users and your bot would be going through that one instance and you would have a concurrency issue where two users talking to the bot at the same time would be storing their values into the same memory (those variables) and stepping on each other's data.

It's also possible, depending on which samples you're following, that you're instead instantiating your GetNameAndAgeDialog on every turn. This would mean that those variables are then initialized to an empty string on every turn of the conversation and you'd lose track of the original values.

Ultimately, regardless of the instancing used, the approach ends up being flawed no matter what when it comes to scale out because at best your state would be pegged to a single server instance and if one turn of the conversation took place on ServerA and the next turn of the conversation took place on ServerM then ServerM would not have the values from the previous turn.

Alright, so clearly you need to store them with some kind of proper state management mechanism. You're clearly somewhat familiar with using BotState (be it conversation or user scope) already being that you're already using the state property accessors, but it's probably premature to store values you're collecting throughout a multi-turn prompt into someplace more permanent until you're at the end of the collection process. Luckily, dialogs themselves are stored into state, which you may have figured out when you set up a state property accessor for DialogState, and therefore offer a temporarily persistence mechanism that is tied to each dialog's lifetime on the dialog stack. Using this state is not obvious or documented well (yet), but WaterfallDialog actually goes a step further and exposes a first class Values collection via its WaterfallStepContext companion class which is fed into each step. This means that each step of your waterfall flow can add values into the Values collection and access values that previous steps may have put into there. There is a pretty good sample of this in the documentation page titled Create advanced conversation flow using branches and loops.

Not Making The Best Use of Prompts

  • You're using a TextPrompt for name which is perfect and you'll get a string from it and be all set. Though you might want to consider throwing a validator on there to make sure you get something that looks like a name instead of just allowing any old value.
  • You appear to be using a NumberPrompt<T> for the age (judging by the name "numberPrompt" at least), but then you .ToString() the step.Result and ultimately do an int.Parse in the final step. Using a NumberPrompt<int> would guarantee you get an int and you can/should just use that value as is rather than turning it back into a string and then parsing it yourself again later.
  • You've got a prompt named "confirmPrompt", but it does not appear to be an actual ConfirmPrompt because you're doing all the Choice work and positive value detection (e.g. checking for variations of "Yes") yourself. If you actually use a ConfirmPrompt it will do this all of this for you and its result will be a bool which you can then just easily test in your logic.

Minor stuff

  • Currently you're using stepContext.Context.Activity.CreateReply to create activities. This is fine, but long winded and unecessary. I would highly recommend just using the MessageFactory APIs.
  • I would always make sure to pass the CancellationToken through to all the XXXAsync APIs that take it... it's just good practice.
  • Your final step either restarts the GetNameAndAgeDialog if they don't confirm the details or starts the MainDialog if they do confirm the details. Restarting with ReplaceDialogAsync is awesome, that's the right way to do it! I just wanted to point out that by using BeginDialogAsync to start the MainDialog means that you're effectively leaving the GetNameAndAgeDialog at the bottom of the stack for the remainder of the conversation's lifetime. It's not a huge deal, but considering you'll likely never pop the stack back to there I would instead suggest using ReplaceDialogAsync for starting up the MainDialog as well.

Refactored Code

Here is the code rewritten using all of the advice above:

public GetNameAndAgeDialog(string dialogId, IEnumerable<WaterfallStep> steps = null) : base(dialogId, steps)
{
    AddStep(async (stepContext, cancellationToken) =>
    {
        return await stepContext.PromptAsync("textPrompt",
            new PromptOptions
            {
                Prompt = MessageFactory.Text("What's your name?"),
            },
            cancellationToken: cancellationToken);
    });

    AddStep(async (stepContext, cancellationToken) =>
    {
        var name = (string)stepContext.Result;

        stepContext.Values["name"] = name;

        return await stepContext.PromptAsync("numberPrompt",
            new PromptOptions
            {
                Prompt = MessageFactory.Text($"Hi {name}, How old are you ?"),
            },
            cancellationToken: cancellationToken);
    });

    AddStep(async (stepContext, cancellationToken) =>
    {
        var age = (int)stepContext.Result;

        stepContext.Values["age"] = age;

        return await stepContext.PromptAsync("confirmPrompt",
            new PromptOptions
            {
                Prompt = MessageFactory.Text($"Got it you're {name}, age {age}.{Environment.NewLine}Is this correct?"),
            },
            cancellationToken: cancellationToken);

    });

    AddStep(async (stepContext, cancellationToken) =>
    {
        var result = (bool)stepContext.Result;

        if(result)
        {
            var state = await (stepContext.Context.TurnState["FPBotAccessors"] as FPBotAccessors).FPBotStateAccessor.GetAsync(stepContext.Context);
            state.Name = stepContext.Values["name"] as string;
            state.Age = stepContext.Values["age"] as int;

            return await stepContext.ReplaceDialogAsync(MainDialog.Id, cancellationToken: cancellationToken);
        }
        else
        {
            //restart the dialog
            return await stepContext.ReplaceDialogAsync(GetNameAndAgeDialog.Id, cancellationToken: cancellationToken);
        }
    });
}

Also is there a way to go back to a spefici waterfall step if ever the user type "back" without restarting the whole dialog?

No, there is not a way to do this today. The topic has come up in internal discussions with the team, but nothing has been decided yet. If you think this is a feature that would be useful, please submit an issue over on GitHub and we can see if it gains enough momentum to get the feature added.

Upvotes: 4

Related Questions