PROGRAMMING

Let's refactor C# code

Flowing SOLID Principles

6 min readOct 13, 2024

restructure (the source code of an application or piece of software) so as to improve operation without altering functionality.
“developers might be asked to review the architecture and, where possible, refactor code to create smaller and more manageable components” — Dictionary Definitions from Oxford Languages · Learn more

As of today, I have been working in the software industry for almost two decades. Writing, reviewing, designing and refactoring the code is & was the primary source of my income. I have been thinking for a while about drafting an article related to refactoring but then not sure what to write then today, I am writing.

There are many gurus on this topic of refactoring and one who inspire me is uncle bob martin who is a promoter for SOLID Principles and clean code.

Software engineer and instructor, Robert C. Martin introduced the collection of principles in his 2000 paper Design Principles and Design Patterns about software rot. The SOLID acronym was coined around 2004 by Michael Feathers. — Wikipedia

The Code

So, let's refactor some code today

using System;
using System.Collections;


class Program
{
   static void Main(string[] args)
   {
       ArrayList myNumbers = new ArrayList();
       myNumbers.Add(1);
       myNumbers.Add(2);
       myNumbers.Add(3);
       myNumbers.Add(4);
       myNumbers.Add(5);

       for (int i = 0; i < myNumbers.Count; i++)
       {
           int number = (int)myNumbers[i];
           if (number % 2 == 0)
           {
               Console.WriteLine("Number is even: " + number);
           }
           else if (number % 2 != 0)
           {
               Console.WriteLine("Number is odd: " + number);
           }
       }
       Console.ReadKey();
   }
}

What does the code do?

It has an Array List, and it contains an item of type integer.
Using for-loop it iterates and with if and else if statement it checks if it is even or odd number.
If even number, it is going to print “Number is even: item” else “Number is odd: item”
Finally, it is going to wait for a key press.

Issues with the code

No namespace exists. It is always better to have a namespace.
Instead of using Array List should have used Generic List of type Integer. Since all the items in the list are integers there is also an unnecessary type conversion (object to integer) happening inside the for-loop it costs in performance & it could have been avoided by introducing Generic List. Array List is of type object so what happens is integer is converted to object and vice versa while retrieving which is an unnecessary overhead which could be avoided with Generic List if we know all items are going to be of same type.
Items are added to the Array List using multiple statements which could be avoided using collection initializers.
Instead of for-loop switch to foreach which is more readable and less prone to errors. The for-loop is based on index and should be used when you want to modify items in it. Yes, performance is slightly better with for loop.
There is an if and else if statement inside the for-loop block but then here we don’t need else if statement, the reason is numbers are either even or odd hence else statement is enough.
Finally, there is a Console.ReadKey(); which is not at all required in modern IDE and can be used unless until it is really required.

Round 1: Refactored code

using System;
using System.Collections.Generic;

namespace RefactoredCodeExample
{
    class Program
    {
        static void Main(string[] args)
        {
            List<int> numbers = new List<int> { 1, 2, 3, 4, 5 };

            foreach (int number in numbers)
            {
                if (number % 2 == 0)
                {
                    Console.WriteLine($"Number is even: {number}");
                }
                else
                {
                    Console.WriteLine($"Number is odd: {number}");
                }
            }
        }
    }
}

You can further refactor the above code.

Round 2: Refactored code

using System;
using System.Collections.Generic;

namespace RefactoredCodeExample
{
   class Program
   {
      static void Main(string[] args)
      {
         List<int> numbers = new List<int> { 1, 2, 3, 4, 5 };
         print(numbers);
      }
    
      static void print(List<int> numbers)
      {
         foreach (int number in numbers)
         {
            if (isEven(number))
            {
               Console.WriteLine($"Number is even: {number}");
            }
            else
            {
               Console.WriteLine($"Number is odd: {number}");
            }
         }
      }

      static bool isEven(int number)
      {
         return number % 2 == 0;
      }
   }
}

Here I have moved the actual logic which identifies if it is even or odd using sub method isEven and another which prints the text if it even or odd to a sub method (print) by this way we have made it generic and can be used for testing with multiple Lists like for example

static void Main(string[] args)
{
   List<int> numbers = new List<int> { 1, 2, 3, 4, 5 };
   print(numbers);

   List<int> numbers2 = new List<int> { 10, 32, 43, 54, 95 };
   print(numbers2);
}

The problem with the above code is, we are trying to test the code using a console output & it is more manual. A better approach would be to introduce unit testing.

Round 3: Refactored code with unit test

using System;
using System.Collections.Generic;

namespace RefactoredCodeExample
{
   class Program
   {
      static void Main(string[] args)
      {
       int[] numbers = new int[] { 1, 2, 3, 4, 5 };
       print(numbers);
      }
    
      static void print(int[] numbers)
      {
         var numberAnalyser = new NumberAnalyser();
         foreach (int number in numbers)
         {
          Console.WriteLine(numberAnalyser.GetEvenOddMessage(number));
         }
      }
   }
  
   public class NumberAnalyser
   {
      public string GetEvenOddMessage(int number)
      {
         return isEven(number)
                ? $"Number is even: {number}"
                : $"Number is odd: {number}";
      }
    
      private bool isEven(int number)
      {
         return number % 2 == 0;
      }
   }
}

Here is what I have done to the code so that it can be unit tested.

Created a new class NumberAnalyser
With GetEvenOddMessage method returning a string. Here it returns “Number is even: {number}” or “Number is odd: {number}”.
It has a private method isEven which is used for identifying if number is odd or even.
You might have a question even for single liner method you have created a method (isEven) and is it really required? not required unless it is being used by another method like GetEvenOddMessage otherwise it is unnecessarily adding complications to the code. Please always follow the KISS principle (Keep It Short & Simple) and please don’t complicate.
Update (16th October 2024): Why are we using generic list & why not integer array? integer array is used when you know the size isn’t going to change & List is used when you are going to add items to it. Here in our case we aren’t adding or removing items hence I have changed it to int[]. We tend rely too much on generic list for anything and everything & thanks to one of my friend for pointing this out hence switched to int[]. The reason why we tend to use it because it gives us many functionalities like Add, Insert, RemoveAt, RemovAll, Clear, Reverse, Find, Sort.. & so on.
Update (16th October 2024): Should we not use for-loop instead of foreach? If we use foreach it internally compiler treats integer array as a special case & it internally optimise and may use for-loop. Also on readability point foreach is more cleaner.

It’s always better to have a code with unit tests in place so that in future if some changes happen the logic won’t break. Here is the sample unit tests for your reference

using NUnit.Framework;
using RefactoredCodeExample;

namespace RefactoredCodeTests
{
    [TestFixture]
    public class NumberAnalyzerTests
    {
        private NumberAnalyzer _analyzer;

        [SetUp]
        public void Setup()
        {
            _analyzer = new NumberAnalyzer();
        }

        [Test]
        public void GetEvenOddMessage_EvenNumber_ReturnsEvenMessage()
        {
            // Arrange
            int evenNumber = 2;
            string expectedMessage = "Number is even: 2";

            // Act
            string result = _analyzer.GetEvenOddMessage(evenNumber);

            // Assert
            Assert.AreEqual(expectedMessage, result);
        }

        [Test]
        public void GetEvenOddMessage_OddNumber_ReturnsOddMessage()
        {
            // Arrange
            int oddNumber = 3;
            string expectedMessage = "Number is odd: 3";

            // Act
            string result = _analyzer.GetEvenOddMessage(oddNumber);

            // Assert
            Assert.AreEqual(expectedMessage, result);
        }
    }
}

Conclusion & Summary

Refactoring the code is fun but then ensure that you don’t complicate your code & please follow the KISS, YAGNI & SOLID Principles. The basic idea is to have code readability, maintainability, testability, and robustness against future changes and future potential bugs.

Update (16th October 2024): More refactoring on the same code is discussed further at https://shiljopaulson.medium.com/lets-refactor-c-code-2-e452465cc503