Good Refactoring / Patterns For Simple UI Logic?

I’ve got a chunk of C# that sits inside of a very simple form. The form downloads an update from a web page – one of two possible downloads, based on which one is available – and shows a message to the user saying it’s done. The basic rules for which one to download are:

  • If a software update is available, download it, highest priority
    • After a software update has been downloaded, exit the app immediately so an external process can unpack it
  • If a data update is available, only download it if a software update is not available
    • After a data update has been downloaded, show the data

As this is a very small application with very simple process and no real business logic, I’ve coded the majority of it straight into the form, in a method that runs in a background worker. The code works and does everything it needs to do, but it’s getting ugly and needs some TLC to refactor it into something more manageable and easier to maintain.

private void DownloadUpdates()
        MessageLabel.Do(ctl => ctl.Text = AvailableUpdate.DownloadingUpdateLabelText);
        //software updates always take precedence over data updates
        if (AvailableUpdate.SoftwareUpdateAvailable)
            this.Do(frm => frm.Close());
            if (AvailableUpdate.DataUpdateAvailable)
                MessageLabel.Do(ctl => ctl.Text = "Complete");
                Cancel.Do(ctl => ctl.Hide());
                ContinueButton.Do(ctl => ctl.Show());
    catch (Exception e)
// error handler code here

(Note: For a description of the .Do() method shown here, see my post on async control updates)

As you can see, there are a few nested if statements in here, some possible duplication, and a generally ugly mix of UI and process. In the past, I would have looked at a few basic options to remove the if statement and clean this up by moving the process into separate classes with events that tell the UI when to update with what information. Since this application is so small, I’m not sure I want to go down that path. There’s not a lot to this, honestly, other than figuring out which file to download and what to do after the download completes.

Given all that – including the small nature of the app – what are the patterns you would move to and suggested refactorings that you would take in this situation? How would you clean up this code, reduce duplication (if it’s reasonable to do so) and make the easier to work with and maintain?

I have several ideas, of course, but I want to see what I can learn from other people’s refactoring and clean up ideas. Post your suggestions to a code sharing site such as Gists or Pastie, then link to them in a comment, here.


About Derick Bailey

Derick Bailey is an entrepreneur, problem solver (and creator? :P ), software developer, screecaster, writer, blogger, speaker and technology leader in central Texas (north of Austin). He runs - the amazingly awesome podcast audio hosting service that everyone should be using, and where he throws down the JavaScript gauntlets to get you up to speed. He has been a professional software developer since the late 90's, and has been writing code since the late 80's. Find me on twitter: @derickbailey, @mutedsolutions, @backbonejsclass Find me on the web: SignalLeaf, WatchMeCode, Kendo UI blog, MarionetteJS, My Github profile, On Google+.
This entry was posted in .NET, C#, Pragmatism, Principles and Patterns, Refactoring. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • Bill Christie

    Here is one option. It keeps it simple.

  • Edin

    When apps are so small, I prefer not to make things more complicated than needed. So in this case, I would leave it as it is. It’s really easy to understand what’s going on…

    In case the app starts growing then you can refactor it to MVP pattern which is great fit for WebForms applications….

  • Bill – nice. that certainly is simple. :)

    i tend to prefer my flow control to be at a higher level / consolidated, though. with that code – though it’s small and easy to read – the flow control is split up between the two child methods. there’s nothing in the higher level method that tells me whether or not either or both of them will not be downloaded or why. i have to piece that together by reading the two methods.

    in this case, though, that’s not a big deal. things are small enough that there’s not much risk. thanks!

  • John C.

    I agree with Edin in that if the code and problem you’re trying to solve are easy to understand, then there’s no need to find a “more elegant” solution. In this case, repeating the code twice is a bit hard to swallow but is tolerable in my experience.

    However, if you find yourself repeating these same lines of code over and over, then I’d probably use events and call them mid download and post download:

  • I think the issue in your code is that the form decides to make updates. I would have an UpdateManager or something like that so that you can interrogate the status of the update, start an update (maybe asynchronously) and then have status update properties that can be bound to your UI.

  • A good start would be extracting the background worker and method into a separate Command class. Then, instead of updating the form directly, just raise an event.

  • This is how I would probably do it: