Alexa Skill Revamp - The Code (the humanity!)

This post is 2 of X, part of the "Battle Cry Revamp" series

Posts in the series I'll udpate the list as I write more:

Alexa Skill Revamp - Battle Cry

Alexa Skill Revamp - The Code (the humanity!)

Alexa Skill Revamp - The Intents

Alexa Skill Revamp - Person Profile

So I opened the project...

I crack open Visual Studio, and my first experience to the old Battle Cry code is...an upgrade wizard. Little unnerving but this could be an old AWS project format, an old .csproj format from before the nice tidy one we have today. It's old - we'll let it slide.

So I open up the solution after the upgrade and I need to start at the beginning, I don't remember this stuff, so I just open the Function.cs file to take a look at the lambda...

...yeah, that was my first mistake (code shortened for brevity)

      this.JourneyCreator = new Dictionary<Journeys, Func<IRequestHandler>>()
      {
        {
          Journeys.ShareCode,
          (Func<IRequestHandler>) (() => (IRequestHandler) new ShareCodeJourney((IShareSkillProvider) new DynamoProvider()))
        },
        {
          Journeys.CreateBattle,
          (Func<IRequestHandler>) (() => (IRequestHandler) new BattleJourney((IBattleProvider) new DynamoProvider()))
        },
        {
          Journeys.Summary,
          (Func<IRequestHandler>) (() => (IRequestHandler) new SummaryJourney((ISummaryProvider) new DynamoProvider()))
        }

Okay. So I wasn't clear on AWS or Alexa at the time - but even so this is some dodgy copy/paste faux pas. But! there's an IRequestHandler, which gives me hope! That is until I notice that there is an IRequestHander file in the solution as well...drat.

Not sure why I care about this interface? Let me explain.

Request Handlers

From a custom skill point of view, a skill is still at it's core a request/response mechanism. No matter how much you rely on the tooling and the APIs and the interfaces at some point your skill is going to get a request and you have to send a response in return.

Now that is a major over-simplification, because requests can be of a particular type. Launch requests are for when your user opens your skill without context, an intent request is when they make a request you recognise, the request might not even really be from your skill invocation - maybe a list you manage had an item added to it? That's a request.

So when you're building a skill you might start off with:

    if skillRequest.Request is LaunchRequest
    ....
    else if skillRequest.Request is IntentRequest

etc., but that's really quickly going to get messy. Also what about where you have optional values in your intent? In Battle Cry we need three defensive moves and three agressive moves, so what if we ask for them and we only get one? Do I want to act the same? Probably not. And you don't want even more complicated logic in your function. So Alexa Skills have Request Handlers, and .NET skills have the Alexa.NET.RequestHandlers package!

This package allows you to seperate this logic into a set of classes implementing an IAlexaRequestHandler which has two methods and in its simplest form looks like this:

public interface IAlexaRequestHandler
{
    bool CanHandle(AlexaRequestInformation<SkillRequest> information);
	Task<SkillResponse> Handle(AlexaRequestInformation<SkillRequest> information);
}

You can then register these handlers as part of an AlexaPipeline object (we'll show an example of that in a little bit). When a new request comes in, it checks the request against the CanHandle methods and the first one that returns true gets to respond to the request. This allows you to arrange more generic handlers lower down the pecking order to ensure that there's always a good response sent back to the user.

There's a lot more to the NuGet package (you can handle exceptions in a similar manner, as well as register interceptors that run regardless of which handler is picked) but that will do for now. So we have a number of what appear to be my initial handler ideas currently in the codebase, but they're based off of an enum? So how...

      if (GeneralInformation.HandledIntents.Contains<string>(intentName))
        return new GeneralInformation();
      if (ShareCodeJourney.HandledIntents.Contains<string>(intentName))
        return this.JourneyCreator[Journeys.ShareCode]();
      if (SummaryJourney.HandledIntents.Contains<string>(intentName))
        return this.JourneyCreator[Journeys.Summary]();

Oooh...yeah no, that's not staying in! Each handler needs to handle its own logic, so I'll refactor this code and use the NuGet package; Each class represents one of my current intents with its intent logic self contained. Just a refactor right now, no functional changes.

So anyhoo - I refactored...

(N.B. This is really difficult! I can already see so much I want to rip out and pretend I never wrote. I know, everyone has nightmares when they open code from years ago - but I REALLY didn't care about the code in this skill, I just wanted to figure out how Alexa fit together at this stage and it shows. So ashamed!)

So what we end up with for each intent is something like this

    public bool CanHandle(AlexaRequestInformation<SkillRequest> information)
    {
        return information.SkillRequest.Request is IntentRequest intent
               && intent.Intent.Name == "BattleCryGetCode";
    }

So much cleaner and more clearly explained about what's going on.

This code is cookie cutter for each intent, and I'm using it to explain the concept more easily and to make the smallest change to my existing code, tbut here are several helper classes in the package - one of which is IntentNameRequestHandler which has a constructor that takes the intent name and does this for you to save the copy paste.

So I've done this to each intent, I've also taken code that was wedged in for handling launch, help, stop and some catch-all code and put them into their own handlers too (it was in a file named GeneralInformation :( ) - required for certification so your alexa skill doesn't throw an exception when the users say something unexpected.

which means the big dictionary copy pastey thing? That code is now just a (very) bad memory, now we have:

    _pipeline = new AlexaRequestPipeline(new IAlexaRequestHandler<SkillRequest>[]
    {
        new Launch(),
        new ShareCodeJourney(provider),
        new BattleJourney(provider),
        new SummaryJourney(provider),
        new FightBackJourney(provider, provider),
        new WarStoryJourney(provider),
        new HelpHandler(),
        new Stop(),
        new CatchAll()
    });

Much cleaner code, I had checks for request types and intent names all over the place - now all covered by the request handlers and seperate to the response building itself. No alteration in functionality, no new functionality added, just tidied up what was there.

N.B. Just in case you're wondering, the reason that SkillRequest is a generic type in some of the declarations? That's because you can override the SkillRequest type if you're using certain extensions - like Alexa.NET.APL, which has extra information specific to the APL aspects of the request, and I'll go into detail in a later post. All the interfaces and classes have default versions that assume SkillRequest and generic versions in case you need them

Also means that big if statement that handled generating the responses? Well that could all go. That is now just:

return _pipeline.Process(input);

Where input is the skil request being sent to the lambda. That's it! I have some error handling code in the lambda that wraps the call in a try catch, just in case - I left that in untouched as I'm trying to be serious about taking this one step at a time. If I were writing this code now I'd implement it as an IAlexaErrorHandler and define it in the pipeline as a second argument. Very similar deal - you order the error handlers and the exception is passed in as another parameter you can test against to give the most accurate response to the user about what's happened.

So what now?

Well I can build the skill now, and I can look at a lot of the code without feeling unwell, which is a big step forward.

While I was moving the code into the requesthandler classes I have noticed that I have a lot of code involved in checking the state of the dialog - what journey I was in to monitor a change, how many moves have been entered, if the values are valid or not, all required at the time.

But the fact is that now I would hazard a guess that a lot of what I've written can be handled within skill management, utilising functionality such as slot validation - for example.

So the next step is to examine the code I've written for that, then go back to the skill definition and see what can be optimised there and hopefully remove even more code to start refining these intents into the functionality they need to be focused on.