Gradually Refactoring Towards Utopia

I can never write perfect code. Over time, an increasing knowledge of the domain, language syntax, design patterns, and even learning entirely outside and irrelevant to the project at hand contribute to an ongoing drift in how I solve problems with (or without) code. At any time, should I have reached a level of happiness with code I’ve just written, the window this level of happiness is sustained for caps out at about three months. At the three month mark I’ll look at code I wrote and see an ill-informed mess.

Now, I love writing code, so, I could just re-write all code every three months. Whilst the desire to refactor is constant and calls me insistently, this approach has no end - I want to implement new features and solve new problems too, and I won’t afford the time to do so should I just re-work old codebases. If I don’t refactor, I could spend too much time implementing changes or new features that could have been delivered quicker should I have refactored towards more maintainable code.

And so a balance must be found where, I’ll refactor only if I have intentions to add further features and changes to the codebase, and, that the refactor will contribute to a decreased time taken to implement these new features. Even so, knowing the refactor will decrease future implementation times is often not possible until after the work is done. In these cases the decision to refactor or not will include weighing my confidence in a new approach and the estimtaed time to execute it, versus my lack of confidence in, and frustration with, the current approach. Performing the refactor then results in either a realisiation of quicker implementation times, or, neglible difference that if caught early I’ll drop the branch instead of going forward.

For the refactors with neglible result caught later - they’ve already become part of the codebase and contribute to the refactor-decision-making-process next time and, remind me I can never write perfect code.

Refactoring Digital Icebreakers

Digital Icebreakers is an open-source project I started 18 months ago. The following sections detail some of the refactoring I’ve implemented over the last week or so. I have two new features I want to implement in the coming days, and a number in the backlog. I hope the following refactors will contribute to an increased feature set for the platform over time. For the record, I know I won’t ever reach code-utopia, but, there can be value in making movements in its general direction.

Moving SignalR to Redux Middleware

During a UI migration mentioned later in this post, I stumbled upon a 5-month-old branch where I had started to move SignalR logic out of components and into a single instance of Redux Middleware. It took a few days to complete this work because testing proved it was incomplete and needed further implementation. The end result obscures SignalR from the rest of the front-end, which is very pleasant from a game-implementation perspective. All SignalR messaging sent to the server and received from the server are defined within the middleware and the only way the rest of the application accesses this message is via standard Redux actions and selectors.

Games no longer need to register SignalR callbacks as this is done automatically by the SignalR Middleware. All that is needed is a reducer to translate incoming state, with the Presenter now being concerned only with how to present selected state:

BuzzerReducer.ts

export const buzzerReducer = createReceiveGameMessageReducer<string, Player[]>(
    Name,
    [], 
    (state, { payload  }) => [
        ...state.filter(p => p.id != payload.id),
        { id: payload.id, name: payload.name, state: payload.payload}
    ]
);

BuzzerPresenter.tsx

export default () => {
    const players = useSelector(state => state.games.buzzer);

    return (
        <ContentContainer header="Buzzer">
            <List>
                { players.map((p) => (
                    <ListItem
                        key={p.id}
                        selected={p.state === 'down'}
                    >
                        {p.name}
                    </ListItem>
                    ))}
            </List>
        </ContentContainer>
    );   
}

Decreasing the size of GameHub

The only SignalR Hub in Digital Icebreakers, GameHub, had grown into a God class. I refactored it and anything that communicated with it (basically the whole backend) such that back-end logic is now separated like so:

GameHub - Predominantly the exposure point for incoming SignalR client calls
LobbyManager - Lobby commands and queries
ClientHelper/Sender - Wrappers for sending messages to clients

It’s pretty easy to keep piling functionality into a SignalR Hub considering it’s the only source of IHubCallerContext. To combat this, instead I inject IHubContext, with the caveat that there will be no caller associated with the invocation. This means ConnectionId and Caller are not available, but I can supply ConnectionId as a method parameter on other classes that are called by the Hub.

Simplifying Client Messaging

While decreasing the size of GameHub I simplified the client messaging. SignalR’s HubConnectionExtensions.SendAsync Method has ten overrides and I was using at least three of them, resuling in difficulty reconciling method calls with the client. Rather than splitting arguments across the overrides, I found it simpler to only use the first:

public static Task SendAsync (this HubConnection hubConnection, string methodName, object arg1);

If I need more propeties sent with the message, I’ll just create a payload object with those properties and pass it as arg1 above. Additionally I’ve moved every instance of sending to clients to the Sender helper class, making it easier to understand the full breadth of Server-Client messaging the platform currently has.

One problem that remains with SendAsync is that the method name is an untyped string. In a future refactoring I can increase quality here further by strongly typing the hub and no longer using SendAsync at all.

Some games on the platform need to display game-specific menu items in the sidebar. Prior to integrating Redux into the project, I was passing chunks of JSX from game implmeentations, through the component tree via callbacks on props, and rendering that JSX via the Sidebar component. Once Redux was integrated, I naievely attempted to to have an action like setMenuItems(items: JSX.Element[]) and while this technically worked, Redux threw heaps of errors in the console as it’s highly recommended you only put serializable things in the store.

I became stumped whilst pondering how I would get some kind of Command pattern into this architecture and eventually realised I was fighting the framework when a post pointed out the Store was the Receiver. I needed the component to be the receiver, or a change in thinking. The solution was a change in thinking - if I moved the component state I wanted to mutate with the Command pattern into the Store, then the component would only be responsible for displaying the result of this mutation.

This approach means the Client and Presenter components don’t even need to know about the menu.

PongMenu.tsx

export default () => {
    const dispatch = useDispatch();
    return (
        <>
            <ListItem>
                <Button onClick={() => dispatch(resetScores())}>Reset score</Button> 
            </ListItem>
        </>
    );
}

PongReducer.ts

export const resetScores = createGameAction(Name, "presenter", "reset-scores");
...
builder.addCase(resetScores, (state) => ({
    ...state,
    score: [0, 0],
}));

Switching Style Frameworks

When I File > New Projected Digital Icebreakers 18 months ago I got bootstrap v3. I pondered upgrading to bootstrap v4, and there might even be a branch somewhere where I attempted it, but I didn’t care enough about it to push through the effort required for a full reskin. These days I’m more familiar with Material-UI and I prefer the CSS-in-JS approach towards styling.

I based the core layout off a free template offered by Creative Tim and after a bit of clean-up and reference fixing, I updated all Digital Icebreakers components to use this new layout and style.

Key differences include: