preloader

Apprentice Dev Blog: Week 3 - Refactoring and Validation

Two people consulting on some code, using a laptop. One person is pointing to a line of code on the screen, whilst another is using the trackpad to move the on-screen cursor

The cover image for this post is by John Schnobrich

This blog post was written by Alex. Alex is our apprentice for 2022, and will be providing frequent developer logs.


Jamie here - to jump directly to Alex’s log for this week, click here

Today marks the completion of Alex’s third week with us.

remember that Alex is NOT their real name

This post actually only covers three days of working and learning on Alex’s project, as this week saw the 2022 Platinum Jubilee. This meant that Thursday and Friday of this week were public holidays here in the UK.

Alex is studying a Digital T-Level course and has joined us for their mandatory eight weeks in industry.

We are incredibly proud to be helping with Alex’s technical education. And as a reminder, I was assigned to be their PM on the project that they are working on and someone to reach out to for help in completing it. The Digital T-Level course that Alex has taken is very comprehensive and covers a number of different development technologies, but there are certain gaps in their knowledge. This is not meant as a negative point towards either the course designers or those who are delivering it, there is simply not enough time to teach all of the things required of a developer in just two short years.

in fact, development is one of the career choices which require a person to be constantly learning new things

With this, and their knowledge from the end of the first two weeks in mind, I decided that we should do some refactoring of the existing code, and double down on input validation for week three.

As a reminder, we started a project with the following goal in mind:

"Let’s start with Framework, then move to modern .NET. And if there’s time, we’ll investigate .NET Maui."

- Jamie

We’re still working with WinForms right now, and that’s primarily because:

  1. Alex has familiarity with WinForms
  2. Modern .NET supports it
  3. (most importantly) there’s 20+ years of documentation and examples out there

Anyone who has had to build a new project will know the pain of not having documentation for their chosen technology. So, for now, we’re sticking with WinForms because there’s 20+ years of documentation, blog posts, Stack Overflow questions, and YouTube videos for Alex to fall back on if I’m not around to help.

One thing which was pointed out by our friend Scott Harden is that:

".NET Maui really is at the bleeding edge, and there isn’t that much documentation out there - and what there is is partially incorrect because it’s so new. My recommendation would be to go WPF first, then .NET Maui."

- Scott

And that’s quite a good point, actually. Especially seeing the number of issues raised against .NET Maui right now

at the time of writing, the number of open issues stands at 1,213

As such, we’ll only migrate the UI to .NET Maui once it’s a little more stable, after stopping by WPF or a web-based technology first.

Anyway, you didn’t come here to read what I have to say. So here is a development log which represents Alex’s third week, in their own words:

one note, I’ve edited for presentation not content


Alex’s Devlog

Hi, I’m Alex and am currently studying a Digital T-Level course, am taking my industry experience with RJJ Software, and these are some of the things that I learned this week.

The first thing we launched into was refactoring. This is where you take the code base as it is and make small changes to increase the readability or accuracy of the code. This is because no project is ever done, and the requirements will change a lot over time. So making sure that the code maintains a good sense of code quality is important.

Sometimes a refactor can take the place of a bug fix or a minor change to the way something is written

Jamie here: it’s important that any refactorings don’t change HOW methods and classes operate from the outside, and that they maintain the same functionality

and sometimes it will be a tidying up exercise. For instance, if there’s a method which takes in a lot of parameters like this:

public void LogNewPatient(string name, string address,
        DateOnly dateOfBirth, int heightInCm,
        List<LogEntry> appointmentNotes,
        List<Prescription> previousPrescriptions)
{
  // do some stuff
}

You can create a new class which encapsulates all of that information and pass that in, making the code a little easier to read:

public class NewPatient()
{
    public string Name { get; init; }
    public string Address { get; init; }
    // etc.
}

public void LogNewPatient(NewPatient newPatient)
{
  // do some stuff
}

Not only does this make the LogNewPatient method easier to read, it also means that you’re less likely to pass the wrong value in (where a lot of them match on type). For instance, in teh old code you could end up accidentally doing something like this:

Jamie here: the following fake data was generated using Fake Name Generator

var name = "Ronald R. Jacobs";
var address = "4641 Eastland Avenue";
var postCode = "WF22 8FQ";
var dateOrBirth = new DateTime(1936, 7, 17, new GregorianCalendar());
var height = 170;
var apptNotes = new List<LogEntry>();
var scripts = List<Prescription>();

LogNewPatient(name, name, postcode, dateOfBirth, height, address, apptNotes, scripts);

It might not be immediately obvious to someone reading the code where the issue lies, but it’s in the second argument passed to the LogNewPatient method. We’ve passed in name where we should have passed in address.

Jamie here: this is a super trivial example, but I like it. It gets the point across really well

After I was done with some minor refactorings, I started to think like a malicious user: how could I break this application?

Well, the app has lots of user input fields, and Jamie taught me that you should never trust user input. He shared this story:

"

A number of years ago, I wanted to order a pizza from a really well known pizza delivery company.

Where I lived was a little hard to get to, and they only gave you 50 characters on their online order form to describe how to get to your house. This wasn’t enough.

I opened the developer tools in my web browser, and snooped around in the page. They were using JavaScript validation, which is fine and does the job, but it’s almost too trivial to get around.

I added a breakpoint to the form submit code, and forced my data to validate by stepping through the code and changing variables as I stepped. Then before sending, I added 1,000 characters of Lorem Ipsum to the delivery instructions field.

It broke their system, because the only validation they thought to add was on the user interface. By "break" I mean that their label printer malfunctioned, because the data it was given didn’t match their template, so my receipt was about six inches longer than it should have been.

The break could have been a lot worse. If they weren’t validating user input correctly, I might have been able to exfiltrate data from their system, or embedded a script which ran at the receipt printing location.

"

- Jamie

So I started adding validation. This is a WinForms application (for the time being), so I added a bunch of if statements on my button click handler:

private void btn_Create_Click(object sender, EventArgs e)
{
    // grab all of the values from the form
    var name = txt_Name.Text;
    var address = txt_Address.Text;
    var dateOfBirth = dp_dateOfBirth.Value;
    var postcode txt_postCode.Text;
    // etc.

    var errors = new List<string>();
    // validate at the UI
    if (string.IsNullOrWhiteSpace(name))
    {
        errors.Add("Name must not be empty!");
    }
    if (string.IsNullOrWhiteSpace(address))
    {
        errors.Add("Address must not be empty!");
    }
    if (string.IsNullOrWhiteSpace(postcode))
    {
        errors.Add("Postcode must not be empty!");
    }
    if (!Over18Years(dateOfBirth))
    {
        errors.Add("Must be older than 18");
    }
    // etc.

    if (errors.Any())
    {
        // show errors here
        // stop, because there were errors
        return;
    }
    
    // send data to the service
    var userRequest = new UserRequest(name, address,
            postcode, dateOfBirth /* etc */);
    _myService.HandleUserRequest(userRequest);
}

private bool Over18Years(DateTime dateOfBirth)
{
    // validate here
}

This worked, but was only UI based validation. We needed to validate in the business logic layer, too. This might seem like we’re doubling the work, but when we swap the UI out for something else, there will already be validation baked into our business logic later, so replacing it on the new UI will be a lower priority.

Because the business logic layer is directly tied to the domain, we should have rules specific to our domain in the validation for the business logic layer. Whereas all validation in the UI should be generic enough, because it has no knowledge of the business logic layer (remember our points about the Single-responsibility Principle from last week).

// We'll skip over validating the name with specific
// rules. But let's say that a user must live in a
// specific area in order to use this system.
// You could argue that that is a domain specific validation
public class ImplementsServiceInterface : IService
{
    private readonly AddressService _addressService;
    
    public ImplementsServiceInterface(AddressService addressService)
    {
        _addressService = addressService;
    }

    public List<string> HandleUserRequest(UserRequest userRequest)
    {
        var errors = List<string>();

        if (!_addressService.UserCanUseService(userRequest.address, userRequest.postcode))
        {
            errors.Add("Based on your location, you cannot use this service");
            return errors;
        }

        // do something with the rest of the data
        return errors; // should be empty here
    }
}

With extra validation in place, I felt more confident that malicious users wouldn’t be able to break my application so easily.

Jamie here: never ever validate names other than "cannot be empty or null". Because all the rules you know about name validation are false.

And because of the 2022 Platinum Jubilee, that’s as far as I got this week.


Jamie again.

One of the things we didn’t get to, but I hope we can start next week is using design patterns. Design patterns are repeatable lumps of code which solve generic problems. I had thought that we could use Alex’s learnings about refactoring and a pattern called the Builder pattern to create the UserRequest models.

Rather than doing something like:

var userRequest = new UserRequest(name, address,
        postcode, dateOfBirth /* etc */);

We could use a builder to do something like:

var userRequest = UserRequestBuilder()
        .WithName(name)
        .WithAddress(address)
        .WithPostcode(postcode)
        .WithDateOfBirth(dateOfBirth)
        //etc.
        .Build();

The benefit of this is that you further reduce the chances of passing the wrong value to a positional argument in the constructor, and you can set individual property validation in the With... methods. It also means that if a user doesn’t supply that value, you don’t have to deal with it in the object constructor.

As we’ve covered most of the SOLID principles by now (with the exception of the Open-closed Principle and Liskov Substitution Principle), the next steps are:

  • look at some common design patterns and see whether they could be of use to Alex and their project
  • now that we have interfaces, look at mocking/faking and building up some unit tests
  • talk about what a unit is
  • look at what we’ll need to do to swap out the UI for another technology

It’s been an incredibly fun ride so far, and Alex is flying through the tasks and learning that they have had to do.