Reducer Performance

[dinoc]

In a larger app where we have lots of reducers and dispatched actions, does it make sense to try and avoid type checking within each reducer?

For example, the following is 1 reducer in the TodoMvc example:

public static ImmutableArray<Todo> TodosReducer(ImmutableArray<Todo> previousState, IAction action)
{
    if (action is AddTodoAction) {...}
    if (action is ClearCompletedTodosAction) {...}
    if (action is CompleteAllTodosAction) {...}
    ...
    return previousState;
}

When an action gets dispatched in a very large app, we would have to type check against every if-statement within every reducer.

Would it be better to add an ActionType enum to the IAction interface? Then the reducer becomes much simpler with a switch statement to check against the ActionType of the IAction instance:

public static LoginState Reduce(LoginState previousState, IAction action)
{
    switch (action.ActionType)
    {
        case ActionType.LoginStarted:
            return new LoginState() { IsProcessingLogin = true };
        case ActionType.LoginSuccess:
            return new LoginState() { IsProcessingLogin = false };
        case ActionType.LoginFailed:
            return new LoginState() { IsProcessingLogin = false };
        default:
            return previousState;
    }
}

Assuming the TodosReducer example did follow a more performance if-elseif-elseif-else flow vs having an if-if-if-if flow it still would be a matter of whether type checking is more performant than checking against an enum, and I would guess the latter is better.

[hungnd1475]

One workaround for this is to use the Visitor Pattern. With this you only have to type cast one time and the Visitor will take care of the rest. Your example can be rewritten using the Visitor Pattern as follow.

First create an IVisitableAction inherited from IAction which can be visited by the Visitor.

public interface IVisitableAction : IAction
{
    LoginState Visit(LoginState prevState, ActionVisitor visitor);
}

Then with each action type you create a concrete class implementing IVisitableAction. Each class can contain their own additional information for the action.

public class LoginStarted : IVisitableAction
{
    public string Username { get; set; }
    public LoginState Visit(LoginState prevState, ActionVisitor visitor)
    {
        visitor.Visit(prevState, this);
    }
}

public class LoginSuccess : IVisitableAction
{
    public LoginState Visit(LoginState prevState, ActionVisitor visitor)
    {
        visitor.Visit(prevState, this);
    }
}

In the ActionVisitor class, add a Visit function for each concrete action class that perform the actual logic for it and return an appropriate state.

public class ActionVisitor 
{
    public LoginState Visit(LoginState prevState, LoginStarted action)
    {
        // here the Visitor can access action's properties
        DoLogin(action.Username);
        return new LoginState() { IsProcessingLogin = true };
    }

    public LoginState Visit(LoginState prevState, LoginSuccess action)
    {
        return new LoginState() { IsProcessingLogin = false };
    }
}

Now you can rewrite the reducer to utilize all this.

public static class Reducer
{
    static readonly ActionVisitor VISITOR = new ActionVisitor();
    public static LoginState Reduce(LoginState prevState, IAction action)
    {
        var visitableAction = action as IVisitableAction; // there is only one type casting here
        return visitableAction?.Visit(prevState, VISITOR) ?? prevState;
    }
}

[dinoc]

I'm not as familiar with the Visitor pattern, which probably makes this feel like it's harder to reason about what is going on. I can appreciate the use of this new pattern in this architecture but it probably won't be for everyone.

[GuillaumeSalles]

Thanks @dinoc for the question.

First of all, it's important to note that clarity was my main focus when I made the reducers in the samples. In my experience, the type checking / casting has never caused a performance issue. Performance issues are often related to doing too much work on the UI thread or the shape of the state.

However, I can explain why I did not add an ActionType property on IAction and how to optimize your reducer.

I can see a major flaw to the enum ActionType. What will be the values of the enum ActionType if it was defined in the Redux.NET library? It's the responsibility of the consumer to define all the possible action types.

The is operator is slower than comparing enums but you will need to dispatch a LOT of different actions type to see the difference. (See this stackoverflow question for a benchmark http://stackoverflow.com/questions/686412/c-sharp-is-operator-performance)

The as operator is more efficient than the is operator + casting, but I choose not to use it for the reducer because it is a little more verbose.

The visitor pattern proposed by @hungnd1475 is a good solution. If you want to simplify it, you could use simple polymorphism.

public asbtract class ActionBase : IAction
{
    public abstract LoginState GetNewState(LoginState state);
} 

public static class Reducer
{
    public static LoginState Reduce(LoginState prevState, IAction action)
    {
        var actionBase = action as ActionBase; // there is only one type casting here
        return actionBase?.GetNewState(prevState) ?? prevState;
    }
}

The disadvantage of this solution comparing to the visitor pattern is that you associate the reducer business login directly to your action. If you use the visitor pattern or the polyformism solution, you lose some advantage of reducer composition given by the functional approach.

Hope it answer your questions !

[GuillaumeSalles]

On a side note, C# 7 pattern matching would be great way to write clean reducers :
https://github.com/dotnet/roslyn/blob/features/patterns/docs/features/patterns.md

[dinoc]

What about adding a simple GetClassName method on the ActionBase and comparing against those in the reducer? That would remove reducer logic from the action.

If you wanted to take that a step further too....creating a new Fody add in to weave a property in for the class name would be kind of cool too.

[dinoc]

Good news...there already is a fody add-in for this - https://github.com/NickStrupat/NameOf

[ahmad2smile]

A bit late to the party But I think simple method overloading can clean up the if-else mess in reducers rather than Dynamic Polymorphism which would add more fuel to fire of redux boilerplate issue.