Tell me if you’ve heard this one before.

A company invests huge amounts of time, money and energy into building an application.

Over time the application gets slower, and slower. Development takes longer, and longer.

“Small” changes ripple through the entire application, causing breaking changes in seemingly random places.

At this point the company realises it’s in a bad place, and decides the only solution is a complete re-write.

Technology has moved on, it will be different this time, we’ll build it properly.

What happens next?

Now there are two versions of the app.

The old, legacy version (which is actually in use) and the shiny, new version (which is not).

As requirements continue to fly in, the development team end up having to implement them in both versions.

So the developers are split into two teams, one to work on the legacy system, one to work on the new.

Naturally, everyone wants to work on the shiny new version.

If you’re really, really lucky, the rewrite might actually get completed, it may even go live and find its way into the hands of real users.

But then the problems really start, because stuff that worked before is missing; crucial business requirements embedded in the code were overlooked during the rewrite.

Slowly, the new codebase descends into a convoluted mess, development slows down.

Perhaps it’s time for another re-write…

complex reality = complex code

The problems here are all too familiar to many of us, but the outcome isn’t surprising; it makes total sense when you stop and think about it.

You see, the complexity in the app wasn’t just because of crap code; it was complex and messy because the business domain is complex and messy.

Asking the same development team to rewrite the entire app requires them to understand all the business logic well enough to represent it in code.

Where does that vital business logic exist? In people’s heads, and in the legacy code.

So now the team have a mammoth task ahead of them; they need to read, decipher and magically re-write all of the existing business logic, using a new framework, in a completely different application.

Copying and pasting from old to new gains little, and rewriting crucial logic carries its own risks…

An alternative - small refactors

So what’s the answer?

Ask a developer who’s been round this hamster wheel a few times and they’ll probably advocate refactoring the code as you go.

“Refactor more” is technically sound, but, in practical terms, vague advice for a team fighting in the trenches every day.

Refactor what?

Refactor how?

Refactoring for the sake of refactoring probably won’t get you anywhere, but refactoring as part of a cohesive strategy just might.

The biggest blocker to introducing new technology into an application (e.g. adopting Blazor instead of WebForms) is nearly always a lack of separation between the UI and the business logic. If it’s not clear where those boundaries are you’re pushing a heavy rock up a very steep hill.

On the other hand, if you can find a way to carve off the UI concerns from everything else, you might just stand a chance of putting a new UI framework on top of the existing business logic…

Here’s a typical WebForms example (from a helpdesk system, based on real code, changed just enough to protect the guilty!)

Search.aspx.cs

public void Search(object sender, EventArgs eventArgs)
{
    dvResults.Visible = true;
    rptTickets.Visible = false;
    pnlError.Visible = false;
    pnlNoResults.Visible = false;
    pnlResultsCount.Visible = false;

    SearchTicketsRequest searchRequest = GetSearchRequestFromUI();

    TicketCollectionResponse searchResponse = 
        _helpdeskService.Ticket.SearchTickets(searchRequest);

    if (searchResponse.IsSuccess)
    {
        if (searchResponse.Tickets.Count() > 0)
        {
            IOrderedEnumerable<Ticket> orderedTickets = searchResponse.Tickets
                .OrderBy(x => x.DateCreated);

            GetUserSummaryForTickets(orderedTickets);

            rptTickets.DataSource = orderedTickets;
            rptTickets.DataBind();

            pnlResultsCount.Visible = true;
            rptTickets.Visible = true;
        }
        else
        {
            pnlNoResults.Visible = true;
        }
    }
    else
    {
        pnlError.Visible = true;
    }
}

Now as legacy code goes, this isn’t too bad. If you ran across this in your code base you’d be tempted to leave it alone.

But this code also represents an opportunity; to slowly refactor your way to a different UI framework.

Imagine you’re tasked with switching this one search feature from WebForms to Blazor.

Which bits are WebForms specific, and conversely which bits would you still keep, even with a different UI framework?

In this case all the references to repeaters, labels, panels etc. are very much WebForms concerns and would stay in this WebForms code behind.

Everything else, if consolidated and moved out of the code behind, could be re-used for Blazor, WebForms, MVC or even Web API…

But this begs the question, what’s a good strategy for this refactor?

Personally (and there is no “correct” answer here), I’d take two simple steps for starters.

  1. Simplify the structure
  2. Separate UI (presentation) concerns form everything else

Simplify the structure

Structure in this context means classes, if statements, services, managers, repositories…

Structure is a double-edged sword in legacy code.

“Good” structure can help you make sense of the code and figure out what’s going on.

Confusing, unnecessary or over-complicated structure obfuscates more than it clarifies, and makes refactoring a lot harder than it might be.

To that end I’d start by inlining some of the calls here; specifically the two calls to methods within this WebForms codebehind, namely GetSearchRequestFromUI and GetUserSummaryForTickets.

public void Search(object sender, EventArgs eventArgs)
{
    dvResults.Visible = true;
    rptTickets.Visible = false;
    pnlError.Visible = false;
    pnlNoResults.Visible = false;
    pnlResultsCount.Visible = false;

    SearchTicketsRequest searchRequest = new SearchTicketsRequest {
        Term = txtSearch.Text
    };
    TicketCollectionResponse searchResponse = 
        _helpdeskService.Ticket.SearchTickets(searchRequest);

    if (searchResponse.IsSuccess)
    {
        if (searchResponse.Tickets.Count() > 0)
        {
            IOrderedEnumerable<Ticket> orderedTickets = 
                searchResponse.Tickets.OrderBy(x => x.DateCreated);

            foreach (var ticket in orderedTickets)
            {
                ticket.User = _helpdeskService.User.ById(ticket.UserId);
            }

            rptTickets.DataSource = orderedTickets;
            rptTickets.DataBind();

            pnlResultsCount.Visible = true;
            rptTickets.Visible = true;
        }
        else
        {
            pnlNoResults.Visible = true;
        }
    }
    else
    {
        pnlError.Visible = true;
    }
}

It turns out these methods weren’t hiding too much code in this case, firstly creating a new search request from the value entered by the user.

SearchTicketsRequest searchRequest = new SearchTicketsRequest {
        Term = txtSearch.Text
    };

Secondly, going off to get additional information about the user for each ticket search result.

foreach (var ticket in orderedTickets)
{
    ticket.User = _helpdeskService.User.ById(ticket.UserId);
}

With these inlined, and on the assumption that the other methods are for data retrieval not presentation concerns, we can think about separating the UI from the logic.

Separating presentation concerns from everything else

What I’m looking for here is the core Request/Response that this code represents.

I find it helps to think of every call made by the presentation layer as being either a query or a command.

In this case, hitting a database to retrieve search results looks an awful lot like a query…

If we consider the request/response for the query it looks like this.

Query Request

  • SearchTerm

Query Response

  • TicketId
  • DateCreated
  • Text
  • User (DisplayName)

I arrived at this definition by exploring the existing WebForms controls, taking each unique piece of information we’re showing in the UI, and representing it in the Query Response.

To use existing code or to not…

It’s tempting at this point to simply stick with the existing TicketCollectionResponse class.

However, in this case I’ve had a look at the real application this example is based on, and TicketCollectionResponse is re-used for many different parts of the system.

As often happens in long-lived code it’s been re-used a few too many times and now couples disparate parts of the system together (any changes to TicketCollectionResponse run the risk of rippling through parts of the code we’re not working on).

With this in mind, we’ll create our new, more specific Request/Response representation and let that delegate down to TicketCollectionResponse (for now).

The plan is to push all the non-presentation code down into our new query, and keep the presentation layer (where we make the request and handle the response) as minimal as possible.

I’m a big fan of MediatR for representing Request/Response patterns like this, so at this point I’d add MediatR classes to represent this query.

Features/Tickets/Search.cs

public class Search
{
    public class Query : IRequest<Model>
    {
        public string SearchTerm { get; set; }
    }

    public class Model
    {
        public bool IsSuccess {get;set;}
        public IEnumerable<SearchResult> Results {get;set;}
        
        public class SearchResult {
            public Guid TicketId { get; set; }
        	public string Text { get; set; }
        	public DateTime DateCreated { get; set; }
        	public string UserDisplayName { get; set; }
        } 
    }
}

Now we can create a handler which will run when we invoke this query,

public class QueryHandler : IRequestHandler<Query, Model>
{
    public async Task<Model> Handle(Query request, CancellationToken cancellationToken)
    {
        throw new NotImplementedException();
    }
}

We’re finally ready to move all the non-presentation code into this handler.

public class QueryHandler : IRequestHandler<Query, Model>
{
    private readonly HelpdeskService _helpdeskService;

    public QueryHandler(HelpdeskService helpdeskService)
    {
        _helpdeskService = helpdeskService;
    }

    public async Task<Model> Handle(Query request, CancellationToken cancellationToken)
    {
        SearchTicketsRequest searchRequest = new SearchTicketsRequest 
        {
            Term = request.SearchTerm
        };
        
        TicketCollectionResponse searchResponse = 
            _helpdeskService.Ticket.SearchTickets(searchRequest);
        
        IOrderedEnumerable<Ticket> orderedTickets = 
            searchResponse.Tickets.OrderBy(x => x.DateCreated);

        var results = orderedTickets.Select(x => new Model.SearchResult
        {
            Text = x.Text,
            DateCreated = x.DateCreated,
            TicketId = x.Id,
            UserDisplayName = _helpdeskService.User.ById(x.UserId)?.DisplayName
        });

        return new Model
        {
            IsSuccess = true,
            Results = results
        };
    }
}

This is everything but the WebForms specific presentation logic, which stays put in the WebForms code-behind.

Search.aspx.cs

public async void Search(object sender, EventArgs eventArgs)
{
    dvResults.Visible = true;
    rptTickets.Visible = false;
    pnlError.Visible = false;
    pnlNoResults.Visible = false;
    pnlResultsCount.Visible = false;

    var model = await _mediator.Send(new Search.Query {
        SearchTerm = txtSearch.Text
    });

    if (model.IsSuccess)
    {
        if (model.Results.Any())
        {
            rptTickets.DataSource = model.Results;
            rptTickets.DataBind();

            pnlResultsCount.Visible = true;
            rptTickets.Visible = true;
        }
        else
        {
            pnlNoResults.Visible = true;
        }
    }
    else
    {
        pnlError.Visible = true;
    }
}

One of the big benefits of adopting MediatR lies in how it declutters your presentation layer.

This WebForms code no longer needs to know about HelpDeskService, TicketService, UserService etc. instead relying on the single IMediator dependency.

When we call _mediator and pass an instance of Search.Query we get a Search.Model response.

Everything we need to drive the UI is contained within that response.

From here we’re free to further refactor the code in QueryHandler, confident that the presentation code doesn’t rely on any implementation details (e.g. TicketService).

We also have a much clearer path to switch UI framework; Blazor could easily make this exact same Mediator call and handle the response.

Discuss on Twitter

Next up