Andy Crouch - Code, Technology & Obfuscation ...

Thoughts On C# Switch Statements

Photo: Michal Mrozek - Unsplash

C# has switch statements much like many other programming languages. While they can be a useful solution in some situations they can also be a code smell.

When should you use a switch statement? When it makes the code’s intention clear or the code easier to read. For example:

if(myEnumeration.Value == 1)
{
  // execute code for value 1

}
else if(myEnumeration.Value == 2 || myEnumeration.Value == 3)
{
  // execute code for values 2 or 3

}
else if(myEnumeration.Value == 4)
{
  // execute code for value 4

}
else if(myEnumeration.Value == 5)
{
  // execute code for value 5

}
else if(myEnumeration.Value == 6)
{
  // execute code for value 6

}
else
{
  // execute code for all other values

}

vs

switch(myEnumeration.Value)
{
  case 1:
    // execute code for value 1

  case 2:
  case 3:
    // execute code for values 2 or 3

  case 4:
    // execute code for value 4

  case 5:
    // execute code for value 5

  case 6:
    // execute code for value 6

  default:
    // execute code for all other values

}

As you can see the switch syntax is cleaner as it removes the need for so many brackets. It also supports “fall through” functionality which makes it easier to see mixed clauses (such as values 2 and 3). I personally like the default label which makes it clear what will be done if no values are matched. Switch statements are ideally suited to enumerations and numeric conditional tests. They are also ideally suited to instances where there is a single execution task based on a successful match.

When should you not use a switch statement? When you are working with non-numeric values. When you have multiple lines to execute on a successful match and when you can see (or smell) an Open Closed Principle violation such as the following code:

public IEnumerable<SubscriptionsModel> ApplySort(IEnumerable<SubscriptionsModel> list, string sortBy, bool asc)
{
  switch (sortBy)
  {
    case "firstname":
      return asc ? list.OrderBy(x => x.FirstName) : list.OrderByDescending(x => x.FirstName);
    case "lastname":
      return asc ? list.OrderBy(x => x.LastName) : list.OrderByDescending(x => x.LastName);
    case "email":
      return asc ? list.OrderBy(x => x.Email) : list.OrderByDescending(x => x.Email);
    case "company":
      return asc ? list.OrderBy(x => x.Company) : list.OrderByDescending(x => x.Company);
    case "typeofuser":
      return asc ? list.OrderBy(x => x.TypeOfUser) : list.OrderByDescending(x => x.TypeOfUser);
    case "telephone":
      return asc ? list.OrderBy(x => x.Telephone) : list.OrderByDescending(x => x.Telephone);
    case "createdate":
      return asc ? list.OrderBy(x => x.CreateOn) : list.OrderByDescending(x => x.CreateOn);
    default:
      return list.OrderBy(x => x.FirstName).ThenBy(x => x.LastName);
  }
}

As you can see this method is applying sorting to an enumerable SubscriptionsModel. Each case line is determining if it should order in ascending or descending order and the field to sort on. The way it is currently written means that if we add a new field into the SubscriptionModel class we would need to update the switch statement. This is far from ideal. An extra point I will make is that there is no case independent comparison here either. This is a reason I tend to not us switch statements with Strings. You have to sanitise the input as we know users can surprise use with unpredictable input.

Code like this is very common. However, C# offers a nice solution to the endless case branches, reflection. Using reflection will allow the above code to become:

public IEnumerable<SubscriptionsModel> ApplySort(IEnumerable<SubscriptionsModel> list, string sortBy, bool asc)
{
  var orderByColumnName = string.IsNullorWhiteSpace(sortBy) ? "FirstName" : sortBy;
  var orderByPropertyInfo = typeof(SubscriptionsModel).GetProperty(sortBy);

  return asc ? 
    list.OrderBy(x => orderByPropertyInfo.GetValue(x, null)) : 
    list.OrderByDescending(x => orderByPropertyInfo.GetValue(x, null));
}

This code uses a PropertyInfo instance which can be used to pass into the OrderBy and OrderByDescending methods. This allows dynamic rather than hard-coded parameters to be used removing the need to change the method when a new field is added. Mixing this into another C# favourite of mine, extensions, you might want to create a generic method such as

public static class EnumberableExtensions
{
  public static IEnumerable<T> Sort<T>(this IEnumerable<T> enumerable, string sortByField, SortOrder sortOrder)
  {
    if(String.IsNullOrWhiteSpace(sortByField))
      return enumerable;
    
    if(sortOrder < 0)
      return enumerable;
    
    var orderByPropertyInfo = typeof(T).GetProperty(sortByField);
    
    if(sortOrder == SortOrder.Ascending)
      return enumerable.OrderBy(x => orderByPropertyInfo.GetValue(x, null));
    
    return enumerable.OrderByDescending(x => orderByPropertyInfo.GetValue(x, null));
  }
}

This is a basic implementation. I have shared this with a mid-level developer recently as a starting point. The challenge was to write an implementation that contains no if statement for the sort direction.

This post has shown that you should think about the way in which switch statements are used. While useful, they need to be used in a way that doesn’t violate the SOLID principles. If you have any feedback or comments around this post then please contact me via twitter or email.

Netlify Manual DNS Configuration

Photo: Open Energy Market

This past week we pushed out a new site at Open Energy Market. Built using Hugo and deployed on Netlify it is the start of an expandable and agile site which we have big plans for.

Normally when I deploy sites on Netlify I migrate the domain nameservers over to Netlify as well. On this occasion, due to our subdomains and internal security, this was not an option. Instead, I had to set up the DNS for Netlify manually. While not hard, it was more buried in their documentation than it could be. I thought I would cover it here as a point of reference.

The first step is to associate your domain with your Netlify site. You do this by going to the Domains section of your site’s configuration. Here you will add your custom domain, www.myfabnewsite.com for example. When you save this then Netlify will also add a record for your naked domain. This is your domain without any subdomains such as www, myfabnewsite.com for example. This is to allow their automatic domain redirects to work.

To get your domain working you need to update both your naked domain and your www subdomains as follows:

  • Update your DNS A record’s Value to 104.198.14.52 which is the Netlify load balancer’s IP address. Your A record will have a @ in the Name field.
  • Update your www CName record to point to the name of your Netlify site. A CName record is used to define subdomains such as www. The record to update will have CNAME in the Type field and www in the Name field. In the shots below (which are obviously not our real details for obvious reasons) you would take the openenergymarket-com-orig name from the Netlify site and add it to the CName record which is showing mysitename.Netlify.com below.

I would suggest that you reduce your TTL value on these records to the minimum value supported. This will reduce the amount of time it takes to test your changes. You can always change them later should you need to when you know it is working.

The final issue we faced once our DNS was set up was that Netlify had only issued a www.domain.com SSL Certificate. Netlify uses the Lets Encrypt authority to provide SSL certificates. It is all done automatically from a button click or when you add your Custom Domain. In this case, it had not created a certificate for the naked domain. It appears that you need to allow the DNS to correctly propagate. Once it has done so you can click Renew Certificate in the SSL section of your site. It then generates a wild card certificate for your domain which covers all variants of it. Naked and subdomains are all secured. It was just slightly frustrating that you have no control over this and the UI feedback was limited.

Now you should have your site running with your domain all secured. If you have any feedback around this post or have trouble with any of the steps then please contact me via twitter or email.

Refactoring Code Violating The Open Closed Principle

Photo: Richard Balog - Unsplash

This post is circling back around on a topic I have covered before, The Open Closed Principle. The Open Closed Principle is part of the Solid design principles. The principle states

“Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification. That is, such an entity can allow its behaviour to be extended without modifying its source code.”

I came across the code I present below this week in a pull request and thought I would cover my suggested change of approach here. The post is also about recognising code smells around the Open Closed Principle and one potential way to fix them.

First, what is a code smell? For first time readers here, Wikipedia states:

“In computer programming, a code smell is any characteristic in the source code of a program that possibly indicates a deeper problem. Determining what is and is not a code smell is subjective, and varies by language, developer, and development methodology.”

So really when people are talking about code smells they are referring to a range of deeper problems. In the case of the code below this was displayed as repeated methods call of identical structure.

private static void RunExpiryReminderService()
{
    try
    {
        var notificationsSent = _expiryReminderService.sendNotifications();
        if (notificationsSent != 0)
        {
             successfulNotifications.Add("MOP/DC Expiry Reminder Service", notificationsSent);
        }

        _consoleWriter.WriteLineWithNewLine("Expiry Reminder Service run succesfully");
    }
    catch (Exception ex)
    {
        failedNotifications.Add("MOP/DC Expiry Reminder Service", ex.Message);
        
        _consoleWriter.WriteLineWithNewLine("Expiry Reminder Service run with errors");
    }
}

private static void RunNotificationsForMetersWithNo_MOP_DC_Data()
{
    try
    {
        var notificationsSent = _customerEngagementService.sendNotificationsForMetersWithNo_MOP_DC_Data();
        if (notificationsSent != 0)
        {
        successfulNotifications.Add("Notifications For Meters With No MOP/DC Data", notificationsSent);
        }
        
        _consoleWriter.WriteLineWithNewLine("Notifications For Meters With No MOP/DC Data run succesfully");
    }
    catch (Exception ex)
    {
        failedNotifications.Add("Notifications For Meters With No MOP/DC Data", ex.Message);
        
        _consoleWriter.WriteLineWithNewLine("Notifications For Meters With No MOP/DC Data run with errors");
    }
}

As you read through these methods it is really clear that each is completing the same task. They run a service method which is a wrapper to execute a query and send the results via email. They add the name of the service method executed if successful to a tracking list. They then write a log entry to the console. If they fail for any reason then they add the name of the service method to a tracking list of failures. They then write a log entry to the console. All very simple and each of the five methods does the same thing (I showed two above). The only difference is the service method call.

A few things instantly stood out:

  • The shape of the methods was identical. This is a similar phenomenon as code smells. If you can spot that two methods are the same shape then it is highly likely that they are doing almost the same thing.
  • The service methods are all parameterless methods that return an integer. This represents the number of records sent in the notification.
  • To extend the application logic for another internal notification new code would be needed. A new service method would be needed and executed in the new management method.

The code was in no way open to extension and did, in fact, require modification for each subsequent change. Given the unique signatures of the service methods, I felt using a Func<int> was an obvious solution.

For the uninitiated C# provides two mechanisms to implement type-safe delegate function pointers. The Func and Action mechanisms can be used to reference and invoke a delegate method. The main difference is that Actions can execute methods that do not return a value. Func delegates are expected to return a value.

Back to the code. Let’s look at one of the methods and break it down.

private static void RunExpiryReminderService()
{
    try
    {
        var notificationsSent = _expiryReminderService.sendNotifications();
        if (notificationsSent != 0)
        {
             successfulNotifications.Add("MOP/DC Expiry Reminder Service", notificationsSent);
        }

        _consoleWriter.WriteLineWithNewLine("Expiry Reminder Service run succesfully");
    }
    catch (Exception ex)
    {
        failedNotifications.Add("MOP/DC Expiry Reminder Service", ex.Message);
        
        _consoleWriter.WriteLineWithNewLine("Expiry Reminder Service run with errors");
    }
}

The first thing each of the repeated execution methods does is to execute the service method. If the service method returns a value then a service method name is added to the successfulNotifications list. If it fails then the service method name is added to the failedNotifications list. Lastly, each method is logging a success or failure message. From this, we can identify the common fields of data required by each repeating method. So to group those fields together we can define a POCO class. So let’s do that

public class InternalNotification
{
    public Func<int> ServiceMethod { get; set; }
    public string Name { get; set; }
    public string SuccessMessage { get; set; }
    public string FailureMessage { get; set; }
    public List<DayOfWeek> ExecutionDays { get; set; }
}

This POCO just defines properties to store the items identified above. It also has an additional property, ExecutionDays, which I will come to later. Now let’s see how these methods were called.

RunExpiryReminderService();

//Services which run weekly

if (DateTime.Today.DayOfWeek == DayOfWeek.Monday)
{
    RunNotificationsForMetersWithNo_MOP_DC_Data();
    RunLoaExpirationNotifications();
    RunNotificationsForMetersWithContractDueToExpiry();
    RunNotificationsForCompaniesWithAllUsersInactivated();
}

So it would seem that some of them should only be executed on a Monday. (For context this code is executed daily). Hence why when defining the POCO above I included the ExecutionDays property. This allows the configuration to outline which days the service methods are run. I cleaned up this logic and added a method to return a list of InternalNotification objects. The method does nothing more than define the InternalNotification objects.

var internalNotifications = new List<InternalNotification>()
{
    new InternalNotification
    {
        ServiceMethod = _customerEngagementService.sendNotificationsForMetersWithNo_MOP_DC_Data,
        Name = "MOP/DC Expiry Reminder Service",
        SuccessMessage = "Expiry Reminder Service run successfully",
        FailureMessage = "Expiry Reminder Service run with errors",
        ExecutionDays = new List<DayOfWeek>(){DayOfWeek.Monday, DayOfWeek.Wednesday }
    }
};

The next thing was to take one of the executing methods and modify it to work with the InternalNotification objects. I also wrapped the code with a foreach loop to execute over a list of InternalNotification objects. This list is generated by filtering the full list of InternalNotification objects by the current day. This means we know we are only executing the notifications for today.

var today = DateTime.Now.Date.DayOfWeek;
var todaysInternalNotifications
       = internalNotifications
            .Where(x => x.ExecutionDays.Contains(today))
            .Select(x => x)
            .ToList();

foreach (var internalNotification in todaysInternalNotifications)
{
    try
    {
        var notificationsSent = internalNotification.ServiceMethod.Invoke();
        
        if (notificationsSent.IsNotZero())
        {
            successfulNotifications.Add(internalNotification.Name, notificationsSent);
        }
        
        _consoleWriter.WriteLineWithNewLine(internalNotification.SuccessMessage);
    }
    catch (Exception ex)
    {
        failedNotifications.Add(internalNotification.Name, ex.Message);
        
        _consoleWriter.WriteLineWithNewLine(internalNotification.FailureMessage);
    }
}

The last step was to remove the old, repeated methods. With that the implementation was complete and we would be able to extend the new code by simply adding the configuration for a new service method to be executed. The processing code would no longer need to be modified to support new notifications.

With the idea scoped out I updated the PR with full details and an explanation of how this approach reduces duplicated methods and addresses the violation. I also extended my comments to recommend that to fully meet the Open Closed Principle the developer should use reflection to load Notification configuration objects based on an abstract interface to define the InternalNotifications. This would allow them to remove the utility method that defines the configuration and the list of notifications.

If you have any thoughts about this post then please contact me via twitter or email.

Observations About If Statements

Photo: Raul Petri - unsplash.com

In this post, I want to highlight a couple of observations about the use of conditional if statements. All languages have a variety of the humble if statement. Programs would not be able to function without them and the logic they provide. Some might say they are the basis of very early AI but I digress.

The basic outline of an if statement is as follows:

if("a specific condition equates to true")
{
    DoSomething();
}
else
{
    DoSomethingElse();
}

This say’s that if the tested condition is true then execute the DoSomething() method. If not execute the DoSomethingElse() method. All pretty straightforward.

But, there are some obvious issues that we should look out for. We do not want to make our code unreadable or hard to understand.

The first observation relates to when many conditions are tested in one go which makes reading the if line a little harder. If reading the conditional test logic is hard then reversing it to understand how you get to the else is even harder.

if((person.Age > 18 && person.Age < 25) && 
   group.Name == "Test Group 1" || group.Name == "Test Group 5")
{
    DoSomething()
}
else
{ 
    DoSomethingElse(); 
}

Vs

var isPersonInTargetAgeRange = person.Age > 18 && person.Age < 25; 
var isATargetGroup = "Test Group 1" || group.Name == "Test Group 5";

if(isPersonInTargetAgeRange && isATargetGroup)
{
    DoSomething()
}
else
{ 
    DoSomethingElse(); 
}

Creating well-named variables and using them in the test condition makes it much easier to read. It is also easier to understand the reverse logic when determining how the else block is reached.

The next problem is the use of deep nested if statements. I have seen a lot of examples of this lately. Code such as:

public string Read()
{
    if(Path.DirectoryExists(_testDirectory))
    {
        if(Path.FileExists(_testFilePath))
        {
            if(!File.IsReadOnly(_testFilePath))
            {
                var file = File.Open(_testFilePath);
                if(file != null)
                {
                    file.Read();
                }
            }
        }
    }
}

The more conditions that are added add a further level of nesting. Code usually ends up like this due to extending requirements. Deep nesting makes it hard to understand the logic that leads to the actual execution of a piece of code. It becomes a lot harder when you have else clauses at multiple levels as well.

public string Read()
{
    if(!Path.DirectoryExists(_testDirectory))
        return;

    var testFileExists = Path.FileExists(_testFilePath);
    var testFileIsReadOnly = File.IsReadOnly(_testFilePath);

    if(!testFileExists || !testFileIsReadOnly)
	return;

    var file = File.Open(_testFilePath);

    if(file != null)
        file.Read();
}

The final issue I want to cover is the use of an if statement to wrap the body of a method. This is another approach which I have seen a lot lately. Something like:

public void AMethod(bool aCondition)
{
    if(aCondition != true)
    {
        LogCondition();
    }
    else
    {
        OpenConnection();
        RequestData();
        ProcessData();
        CloseConnection();
    }
}

Why is the else used? Why not just add a return in the if block and remove the else? The reverse of this is even more strange:

public void AMethod(bool aCondition)
{
    if(aCondition == true)
    {
        OpenConnection();
        RequestData();
        ProcessData();
        CloseConnection();
    }
    else
    {
        LogCondition();
    }
}

Why? Why would you write this? You should look to exit your functions and methods at the earliest opportunity. This is why approaches such as parameter checking are available in most languages. There are patterns that mean you can check and exit out if you get a null or unexpected value. Wrapping your method body like either of these examples is a clear signal you need to refactor your code.

public void AMethod(bool aCondition)
{
    if(aCondition != true)
        LogCondition();

    OpenConnection();
    RequestData();
    ProcessData();
    CloseConnection();
}

if statements are a core part of most programming languages but you need to ensure their use does not result hard to understand code. If you find yourself nesting if statements or if you find that your test conditions span multiple lines then you almost certainly need to refactor your code.

If you have any thoughts about this post then please contact me via twitter or email.

Improving SEO For Your Jekyll Site

Photo: Victor Hughes - unsplash.com

Search Engine Optimisation (SEO) refers to how you increase the quality and quantity of organic traffic to your web site. There are many articles and tools available covering best practices. I would suggest this article to start with and then research the various points. Getting your sites SEO right is critical. It will increase your chances of people finding your content in search engines.

Working on a new site for Open Energy Market has made me investigate how well this blog is optimised. One of the first things I wanted to improve was its SEO score on web.dev. Before making the changes in this post the score started at 49.

I have written before about how I build this site with Jekyll. After a little Jekyll SEO optimisation research, I discovered the Jekyll SEO Tag gem. The gem automatically generates recommended SEO meta tags for each page and post in your site. The following are some of the tags created as part of the configuration and content of the site:

  • Page title. This will be generated with the site title and/or description appended.
  • Page description. If you use a description tag in your forward matter this will be generated otherwise it will be skipped.
  • Canonical URL. The fully qualified URL to your post or page.
  • Next and previous URLs on paginated pages. When generated on a paginated page, the gem will include the URLs for the next and previous pages.
  • JSON-LD Site and post metadata for richer indexing. This moz.com article covers the use of this tag,

See the Github documentation for a full list of tags that can be configured and generated.

To install jekyll-seo-tag you need to add the following to your Gemfile:

$ gem 'jekyll-seo-tag'

Next, you need to add the plugin in your sites _config.yml:

plugins:
  - jekyll-seo-tag

And finally you need to add the following on the line before your </head> tag:

{ % seo % }

(Without the spaces between the brackets and the % character. Shown as such here to prevent Jekyll from compilling the command) e.classList.remove(“lazy”); Once these settings are added you can run:

$ gem install jekyll-seo-tag

That will install the plugin. Once you start your site back up and view the page source you will see the tags and the content generated by the plugin. On re-running web.dev my SEO score had increased to 92. This is without adding specific keywords or any other changes.

I will write up in a future post other optimisations for SEO as I decide on the ones to implement. For now, the little work it took to add this useful plugin is well worth it.

If you have any suggestions or questions about this post then please contact me via twitter or email.