Wednesday, March 09, 2022

What's wrong with this WinUI code?

Here's some simple code.

There are probably many ways you can see that it could be different, but there's one major potential problem with this code. Can you see what it is?

private async Task OpenInBrowserAsync(Uri uri)
    await Windows.System.Launcher.LaunchUriAsync(uri);

The problem with this code is that it contains an unprotected "seam" between the code that's part of this codebase/application and code that isn't.

I'm going to use "your code" to mean the code that's written by you and "other code" for everything else.

Anywhere there's a place where your code meets other code, it creates a seam. A seam is where there's a potential weakness. When two things are connected or joined together, there's a potential for problems. It's where defects or issues are most likely to surface.

Let's go back to the code above. When LaunchUriAsync() is called, three things could happen.

  1. The URI could be opened in the default browser. This is what is expected and will hopefully occur most of the time.
  2. There could be an exception thrown by the other code.
  3. Something could happen that means the provided URI can't be opened, but no exception is thrown.

(Yes, other possibilities exist but won't be covered here.)

Scenario 1 is the easiest to think about. Everything works as expected, so we don't need to discuss this further.

Scenario 2 is vital to consider as it's likely to have a significant negative impact on the people using your app. Hopefully, it'll never happen, but it's always a possibility. You don't want other code to cause your app to crash. The people using it will still blame you. That an exception has its source in other code doesn't matter. That any potential exception is also coming from async code has the potential to increase the possibility that the exception could crash the whole app. Handling the potential for other code to throw an exception is essential!

Scenario 3 is easy to overlook. That code "works" or throws an exception is easy to appreciate. That there's a middle ground--where it "works" but not as expected--can be harder to appreciate and allow for. However, it's equally important for the people using the app that you allow for this as it is to handle exceptions.
If someone uses an app and when they do something, it causes the app to unexpectedly crash, it's easy to know something is wrong. But what happens when the person does something (click a button, select an option, whatever..) and nothing happens? The thing they wanted or expected doesn't happen, but neither does anything else? Do they try and do the action again? Could that have separate, adverse side effects? Is it being slow? Or slower than usual/previously? Did they do something wrong? Is the app broken?
Creating uncertainty in the minds of the people using your app is not something you want to do. It does not lead to happy users. Unhappy users lead to less usage, fewer sales, fewer recommendations, bad reviews. Nothing good.

As per the documentation, the method returns a boolean value to indicate if the URI could be launched.
Ignoring responses is not always wise and is certainly not a "best practice."

In programming, it's usually good to call a method and trust it will do the thing that's asked of it. However, a function that returns an indication of whether it could do the thing that was asked of it is not to be ignored.
Imagine asking someone if they can do something for you but sticking your fingers in your ears when they say if they can do it or not. You could assume that they can and will do it for you. But, if they say they can't, and you carry on assuming they can (and will), whose fault is it when it doesn't get done?

Do you like ambiguous code? Do you like using software where you can't be sure if it's working as expected? Do the people using your app like software that works like this? If not, don't give it to them.

Connected with the above is the issue of testability.

As the above is a small snippet of a larger codebase, it's not clear how (or if) it is tested. But we can assume it probably isn't.

It might be appropriate to abstract the interaction with the other code to make it possible to easily (and automatically?) test different actions and responses. Such tests can be more important than tests related to your own code, as it's often harder (or impossible) to forcibly get every possible response from other code so it can be tested.

So, what should the above method actually look like?

Well, here I might disappoint you. There is no single "best practice" way to write a version of the above method.

Instead, a "leading practice" would be to handle the response from the method, acknowledge that an exception is possible, and make it possible to test for such scenarios.
Maybe I'll show you an example of such code in the future. Let me know if you want to see it.

The above doesn't only apply to code that is part of the WinUI API.

This applies to any code that you didn't write. It might be an API from the platform, or it could be any library you're using. Any "seam" between code you write and code you didn't needs to be protected.

This doesn't (hopefully-obviously) only apply to WinUI related code, but I found a variation of the above amongst code that was supposed to be an example of "best practices." In lots of ways, it's definitely not a good example but I called this out in relation to WinUI as developers building with it deserve the best help I can give.


Post a Comment

I get a lot of comment spam :( - moderation may take a while.