3
\$\begingroup\$

My idea is I can define a block of code (inline) that corresponds to an input string then can call that block of code when given that input.

I'm used to this sort of structure in JavaScript like so:

var lookup = {
  "foo": function() { ... },
  "bar": function() { ... },
};

lookup["foo"];

And I'm looking for a C# equivalent. So far I've come up with the following and it seems to work, but I don't know if it could be simplified or improved upon.

class CortanaFunctions
{
        /*
        This is the lookup of VCD CommandNames as defined in 
        CustomVoiceCommandDefinitios.xml to their corresponding actions
        */
        public readonly static Dictionary<string, Delegate> vcdLookup = new Dictionary<string, Delegate>{

            /*
            {<command name from VCD>, (Action)(async () => {
                 <code that runs when that commmand is called>
            })}
            */

            {"OpenToDoList", (Action)(async () => {
                StorageFile file = await Package.Current.InstalledLocation.GetFileAsync(@"ToDo.doc");
                await Launcher.LaunchFileAsync(file);
            })},

            {"OpenReddit", (Action)(async () => { 
                 Uri website = new Uri(@"http://www.reddit.com");
                 await Launcher.LaunchUriAsync(website);
             })},

        };

        /*
        Register Custom Cortana Commands from VCD file
        */
        public static async void RegisterVCD()
        {
            StorageFile vcd = await Package.Current.InstalledLocation.GetFileAsync(
                   @"CustomVoiceCommandDefinitions.xml");
            await VoiceCommandDefinitionManager
                   .InstallCommandDefinitionsFromStorageFileAsync(vcd);
        }

        /*
        Look up the spoken command and execute its corresponding action
        */
        public static void RunCommand(VoiceCommandActivatedEventArgs cmd)
        {
            SpeechRecognitionResult result = cmd.Result;
            string commandName = result.RulePath[0];
            vcdLookup[commandName].DynamicInvoke();
        }

 }

VCD file:

<?xml version="1.0" encoding="utf-8" ?>

<VoiceCommands xmlns="http://schemas.microsoft.com/voicecommands/1.2">
  <CommandSet xml:lang="en-us" Name="CustomCommands">
   <CommandPrefix> listen here </CommandPrefix>
    <Example> open to do list, open reddit</Example>


    <Command Name="OpenToDoList">
      <Example> open to do list </Example>          
      <ListenFor> open to do list</ListenFor>    
      <Feedback> opening your to do list</Feedback>
      <Navigate/>          
    </Command>

    <Command Name="OpenReddit">
      <Example> open reddit </Example>          
      <ListenFor> open reddit </ListenFor>    
      <Feedback> opening reddit </Feedback>
      <Navigate/>          
    </Command>
  </CommandSet>
</VoiceCommands>
\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

A problem with the code is that the async lambdas in the dictionary are actually async void.

Async void methods have different error-handling semantics. When an exception is thrown out of an async Task or async Task<T> method, that exception is captured and placed on the Task object. With async void methods, there is no Task object, so any exceptions thrown out of an async void method will be raised directly on the SynchronizationContext that was active when the async void method started. -- Async/Await - Best Practices in Asynchronous Programming

It's also confusing because vcdLookup[commandName].DynamicInvoke() looks like a blocking call, but it isn't. Consider this code:

private static readonly Dictionary<string, Delegate> vcdLookup = new Dictionary<string, Delegate>
{
    { "OpenToDoList", (Action)(async () =>
        {
            Console.WriteLine("Opening to-do list...");
            await Task.Delay(TimeSpan.FromSeconds(1));
            Console.WriteLine("Opened to-do list");
        })
    }
};

public static void RunCommand(string command)
{
    vcdLookup[command].DynamicInvoke();
    Console.WriteLine("Finished command");
}

If we call RunCommand("OpenToDoList"), the output is

Opening to-do list...

Finished command

Opened to-do list

I would recommend instead to change vcdLookup to a Dictionary<string, Func<Task>>.

private static readonly IReadOnlyDictionary<string, Func<Task>> vcdLookup = new Dictionary<string, Func<Task>>
{
    { "OpenToDoList", async () =>
        {
            StorageFile file = await Package.Current.InstalledLocation.GetFileAsync(@"ToDo.doc");
            await Launcher.LaunchFileAsync(file);
        }
    }
};

public static async Task RunCommandAsync(string command)
{
    SpeechRecognitionResult result = command.Result;
    string commandName = result.RulePath[0];
    await vcdLookup[commandName]();
}

A couple of other notes:

  • Async methods should have the suffix Async, by convention
  • You probably don't want to make vcdLookup public, exposing it to the world
  • You can declare vcdLookup as an IReadOnlyDictionary so that its contents are not modified after creation
\$\endgroup\$
0
0
\$\begingroup\$

I'd avoid mapping string values to instructions in an object-oriented language like C#. I think the approach can be solid given the use case, it's just that I'd do it in F# instead. Try declaring the async functions themselves internal to your F# library, then have a public F# method that takes in a key and returns the task used to await the async method specified. Since you don't have to worry about return types this works as-is. If you need to start dealing with return types you'll need to create an object to wrap your results.

If F# is out of the question, then having an intermediate dictionary would probably be overkill; since you're imperatively invoking these methods statically, I'd just create a handler object with a method containing a switch that calls the desired method (and I'd change the code to start returning awaitable tasks). With the dictionary, it may seem cleaner, but really maintenance requires following the same chain of events and now developers have to jump around between areas of code due to the increased cyclomatic complexity.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.