Monday, March 14, 2016

Breaking out nested if conditionals into pure function

One common pattern of code is a series of nested ifs. Inside the conditional block you may do some stuff. I was thinking about how to break how functional pieces of code so I was thinking about how to refactor such a piece of code to move it towards pure functions and easily testable code. I have picked out a random piece of code from a project I work on:

var viewMode = false;
var canView = true;

if (!SecurityManager.UserHasPermission(new SecurableIdentifier("0",
    (int)ObjectTypeIds.Category), (int)PermissionIds.View)) 
{
    canView = false;
    if (!string.IsNullOrEmpty(categoryId) 
        && SecurityManager.UserHasPermission(
            new SecurableIdentifier(categoryId, (int)ObjectTypeIds.Category), 
            (int)PermissionIds.View))
    {
        viewMode = true;
        canView = true;
    }
    else if (!SecurityManager.UserHasPermission(new SecurableIdentifier("0",
        (int)ObjectTypeIds.Category), (int)PermissionIds.Create))
    {
        throw new PermissionDeniedException();
    }
}

Now this isn't too bad. It is a random sample and not picked for badness. I want to increase the testability of this as well as making it more self-documenting. My approach is to isolate the conditional logic into a function. I will build one function that will determine what case you are in. This function will return an enum.

private enum SecurityState 
{
    ViewPermission,
    NoViewPermission,
    PermissionToViewSpecificCategory,
    NoCreateOrViewPermission
}

Now I have a nice little enum. The names may not be great, but now I can build a function very much like the above code.

private SecurityState FigureSecurityState(string categoryId = null)
{
    if (!SecurityManager.UserHasPermission(new SecurableIdentifier("0",
        (int)ObjectTypeIds.Category), (int)PermissionIds.View)) 
    {
        if (!string.IsNullOrEmpty(categoryId) 
            && SecurityManager.UserHasPermission(
            new SecurableIdentifier(categoryId, (int)ObjectTypeIds.Category), 
            (int)PermissionIds.View))
        {
            return SecurityState.PermissionToViewSpecificCategory;
        }
        else if (!SecurityManager.UserHasPermission(new SecurableIdentifier("0",
            (int)ObjectTypeIds.Category), (int)PermissionIds.Create))
        {
            return SecurityState.NoCreateOrViewPermission;
        
        else
        {
            return SecurityState.NoViewPermission;
        } 
    
    else 
    {
        return SecurityState.ViewPermission;
    }
}

This isolates the conditional logic and allows you a variable to describe the condition. Another way to get this benefit is just using a named variable before each if to describe the condition. This approach has the added benefit is you can test just the condition logic. Now you can write a flat conditional structure with very clear syntax:

var securityState = FigureSecurityState(categoryId);

switch (securityState) {
    case SecurityState.PermissionToViewSpecificCategory:
        ...
    break;
    case SecurityState.NoViewPermission:
        ...
    break;
    ...
}

I didn't finish the code, but I think what I wrote conveys a sense of what you would do. This refactoring pattern isn't always the best thing to do, but it is good to have it in your toolset when tackling a set of nested conditions.

Another way to handle large blocks of nested conditions is to simply make each block it's own function. Now how you handle this issue depends on the code and the nature of the problem. I mainly came up with this approach as a way to factor out nested conditional logic into a pure function.


No comments:

Post a Comment