Refactoring in C#: Notes From a Pluralsight Course

Posted in C# Programming, I Learned This Today

What I Learned Today

This course was a great overview and intro to the world of refactoring. The beginning of the course went into the reasons for when to refactor and when to leave code alone. 

It was created by a man named Steve Smtih who is located at this website: http://ardalis.com/ You can find the course here: https://app.pluralsight.com/library/courses/refactoring-csharp-developers/table-of-contents

First, let’s define the word we will be discussing. 

Refactoring is defined as “a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its behavior.” Simple enough of a definition. The course continues by diving into this umbrella term. 

What’s the Process for Refactoring?

There is a simple process of what to do when attempting to refactor. This was something I hadn’t thought about when I refactored code myself. 

  1. Make sure to commit all changes made to the branch before refactoring so you have a restore point if things don’t go well.  
  2. Verify the functionality of the current code by running automated tests. 
  3. Apply the refactored code. 
  4. Confirm that the same functionality works with the automated tests. 

These steps seem very straightforward, but I want to talk more about the 2nd and 4th step. Not everyone, myself included, have a suite of automated tests to run for each page on their project. There is a solution to this that can be easy to do and saves yourself time in the future; characterization tests. 

Characterization tests are tests that essentially reflect the current behavior of the system. It’s a test that asserts that the system does what it says it does. 

Steps for Creating a Characterization Test

  1. Write a test that you know will fail. 
  2. Use the failing test output to capture the current behavior.
  3. Update the test to assert the current behavior. 
  4. Run the test against the newly refactored code and it should pass. 

Something to note when writing tests, make sure to write a variety of tests against the methods you are refactoring. That way you are able to have a bigger safety net to catch any unwanted changes that happen due to your refactoring. 

In my own experience, I can think of a moment when I had refactored code to simplify when cells in a grid would color based on a specific number. What I ended up doing was adding in a bug for whenever a value was empty, it would default to one of the colors instead of showing up empty. An easy fix but I didn’t see it until it was out in production so you can see how a test would have been helpful here. 

Classification of Code Smells

The next question to discuss is how a developer can know when their code might need a refactor. There is a term that gets used in refactoring a lot called code smell. One definition is “a surface indication that usually corresponds to a deeper problem in the system.” This term was first published in Martin Fowler’s book, Refactoring. https://martinfowler.com/bliki/CodeSmell.html 

The main benefit of having names for code smells is to allow developers to have a common vocabulary to reference when there are refactors in code. So when I make a commit that deleted commented out code, I can simply refer to the code smell of “dead code” to communicate my intention for taking the code out. 

This does not in any way mean that code smells are bugs or need to be followed in every case they are found. They are simply a tool to use in preventing more work down the road.

The following are categories that the different code smells can fall into. The terms are not necessarily used often but it is still good to keep in mind. I list the name followed by some aspects of the category. 

  • Bloaters
    • Makes the codebase bigger than necessary.
    • Usually impacts the code slowly over time. 
  • Object Orientation Abusers
    • Breaks polymorphism. 
    • Create inappropriate tight coupling. 
    • Requires repetition. 
  • Change Preventers
    • Touches many parts of the system.
    • Creates tight coupling.
    • Lack of separation of concerns. (SOLID Principle)
  • Dispensables
    • Provide little or no value.
    • Can be safely removed with no recourse.
  • Couplers
    • Introduces excessive coupling. 
    • Tie unrelated parts of the system together. 
  • Obfuscators
    • Impedes clear communication. 
    • Hide intent. 
    • Confuses the reader. 

The course now goes into examples of the most common code smells. We will look at smells at a statement, method, and class level. 

Statement Code Smells

The following are low-level code smells along with techniques to address them. 

Smell: Primitive Obsession | Category: Bloater

Definition: Overuse of primitives instead of better abstractions or data structures. It results in excess code. 

Example: 

  • Bad: Add.Holiday(7,4); 
    • Looking at this, we don’t know which field is for the month and which is for the day. Also, this date might not be a holiday all around the world. 
  • Good: Date independenceDay = new Date(7,4) 
    • Add.Holiday(independenceDay); 
    • This is better but we still aren’t sure what date it represents. Is it July 4th or April 7th? 
  • Better: Date independenceDay = new Date(month: 7, day: 4);
    • Here we can see easily that the intention is to have the date of July 4th without having to dive into the function. 

Smell: Vertical Separation | Category: Obfuscator

Definition: When code goes against the techniques below. 

Fix: 

  • Define, assign, and use variables and functions where they are used.
  • Define local variables where the first user ideally as they are assigned.
  • Define private functions just below their first user. Avoid forcing the reader to scroll. 

Smell: Inconsistency | Category: Obfuscator

Definition: Inconsistent in naming, formatting, and usage patterns within the application. 

Fix: Be consistent!

Smell: Poor Names | Obfuscator 

Definition: Naming things has often been cited as one of the hardest problems in computer science. 

Fix: Use descriptive names and avoid abbreviations and encodings when possible. For more, on naming conventions *ADD LINK* read my take on the chapter from Martin Fowler’s book Clean Code. You can also check out Microsoft’s coding conventions here.

Smell: Switch Statements | Category: Object-Orientation Abuser

Definition: Switch statements, and complex if-else chains, may indicate a lack of proper use of object-oriented design. 

Fix: There are other ways around using a switch statement or complex if-else statements. I explain one way through an abstract factory in this article on clean code functions. 

Smell: Duplicate Code | Category: Bloater

Definition: Having duplicate code violates the D.R.Y. principle. It can be said to be the root of all software evil. 

Fix: Don’t use copy-paste code. Find a way to limit repeating functions or methods. The less duplication, the clearer, and more concise the code can be read. 

Smell: Dead Code | Category: Disposable

Definition: Get rid of useless code that is never executed. It’s not adding any value. It only adds weight to the codebase and it’s a distraction to the reader. 

Fix: Delete, remove, and bury it.

Smell: Hidden Temporal Coupling | Category: Coupler

Definition: Certain operations must be called in a certain sequence, or they won’t work. Nothing in the design forces this behavior, developers just have to figure it out from context or group knowledge. 

Example: Say you make the process of baking a cake into separate functions. You might have functions such as PrepareIngredients(), PreHeatOven(), Bake(), AddToppings(). Now, if you go out of order by calling these functions, the cake will not turn out very well. 

Fix: Here are a couple of ideas for fixing the issue. I hadn’t thought of this one before so this made an impact in my code a lot. 

  1. Using an abstract class to enforce the sequence order.
    1. Create an abstract class called BakedCakeBase. 
    2. Create a method called MakeCake() which will invoke the needed functions in order. 
      1. PrepareIngredients(), PreHeatOven(), Bake(), AddToppings()
    3. Have the class that calls this function to implement the abstract class and then override the functions called. 
  2. Make each function rely on the output of the function before. 
    1. var preparedIngredients = PrepareIngredients();
    2. var preHeatedOven = PreHeatOven(preparedIngredients);
    3. var bakedCake = Bake(preHeatedOven);
    4. var finishedToppings = AddToppings(bakedCake);

Method Code Smells

Smell: Long Method | Category: Bloater

Definition: The title gives it away. The idea needs to be to prefer to create small methods to longer methods. 

Benefits of Small Methods: They can have better names since they are doing less and they are easier to read and understand since they aren’t doing as much. 

Fix: If you run into longer methods, try to see if it is doing multiple things and extract one of the additional tasks into a separate method. 

Example: When the code needs to be checked for null, instead, use guard classes to check the values rather than adding the check in the function.  

Smell: Conditional Complexity | Category: Change Preventer

Definition: Methods should limit the amount of conditional complexity they contain. The number of unique logical paths through a method can be measured as Cyclomatic Complexity, which should be kept under 10. Note! This number refers to how many PER method; not the entire page. 

Fix: I personally have not heard of this before now, but built into visual studio there is a way to analyze your code to see the Cyclomatic Complexity rating for each function. This isn’t a typical thing that occurs in your code but it’s another way to see if your code may need a refactor. 

Smell: Inconsistent Abstraction Level | Category: Change Preventer

Definition: Methods should operate at a consistent abstraction level.

Fix: Don’t mix high level and low-level behavior in the same method.  

Example: If the method in question is dealing with UI (user interface) concerns, it should not be also performing a database call in the same method. Let another function worry about the database call and focus on the UI in the UI method. 

This gives a high-level view of some of the issues that can slowly creep up from the code. I did not go into detail on the different strategies for dealing with these code smells so if you find some of these smells in your own code, make sure to look up different ways of handling the situations. 

Class Code Smells

Smell: Large Class | Category: Bloater

Definition: A large class probably has too many responsibilities and can be split into two or smaller, more focused, and cohesive classes. 

Fix: If the class is too large, extract it into smaller classes. This is similar to the idea of small functions. It’s easier to have a tool shed with a lot of drawers holding specific tools rather than large drawers holding multiple tools. 

Smell: Class Doesn’t Do Much | Category: Bloater

Definition: Sometimes after refactoring or redesign, a class is left without much to do. If whatever it does makes more sense somewhere else, pull it out and delete the class. 

Fix: Make sure classes have value and are not just taking up space. 

Small: Temporary Field | Category: Object-Orientation Abuser

Definition: A class has a field that is only set in certain situations, often used to pass state between methods instead of using parameters. 

Example: 

If a field is a private global and someone expects it to be set but does not know they need to set it themselves, they will run into issues.

Fix: 

We want to make the field that would have been a private global into the function that sets it.

Smell: Alternative Classes with Different Interfaces | Category: Object-Orientation Abusers

Definition: Common operations should share common semantics, such as names, parameters, and parameter orders. 

Example: If you have two methods that send emails and one of the parameters take (“to”, “from”, “subject”, “body”) and another that takes (“from”, “to”, “subject” “body”), you will likely have issues when they get mixed up. 

Smell: Data Class | Category: Dispensable

Definition: Classes that lack behavior and have only fields and/or properties. Such classes lack encapsulation and must be manipulated by other classes, rather than bundling state and behavior together. 

Example: If we have a method that returns a list and we allow that method to interact with the list itself, that method could call list.Clear() at any point in time with the list instantiated which could be difficult to debug and find. 

Fix: We need to create the methods in the data class itself to encapsulate the data and give it out in a controlled fashion. 

Smell: Feature Envy | Category: Coupler

Definition: This occurs when behavior lives in one object but requires data from another. This is related to Data Class, but can also occur between two classes that each have their own behavior. This might violate the “Tell, don’t ask” principle. 

Fix: If the classes are performing similar functions, think about combining them into one. Otherwise, make note of the possible issue with the setup of the classes.

Smell: Hidden Dependencies | Category: Coupler

Definition: Classes have extended dependencies they do not specify through their constructors. The calling code must inspect the class (or discover runtime errors) to identify its dependencies. Something to keep in mind is “new is glue”, meaning that wherever “new” is called, the class is stuck to that dependency and its changes. 

Fix: Instead, follow the Explicit Dependencies Principle.

Fix2: Another way around this is to use dependency injection into the constructor. This solves the issue of not knowing what the class depends on.

Specific Class and General Refactorings 

Some things to keep in mind when refactoring that the course mentioned. 

  1. Encapsulate Fields 
    1. Any classes that have a public field should instead have it as a property.
  2. Encapsulate Collection
    1. Don’t allow things to directly add items to a collection but make them use an add/remove method. 
    2. This can be done by returning an IReadOnlyCollection<>
  3. Move Method
    1. If a method references more of another object than the one they are in, move it!
  4. Extract Class
    1. If some data and methods work well together, extract it into its own class rather than adding more responsibility to the one it’s in. 
  5. Replace Inheritance with Delegation 
    1. Instead of inheriting a class called Employee, why not delegate it as private Employee _employee. That way there is more abstraction. 
  6. Replace Conditional with Polymorphism
    1. If there are a lot of “if” statements or switch statements, add it as an abstract to the base class. This relates back to creating abstract factories that I mentioned in the clean code with functions post. 

So that about wraps up the general details of this course I took. It was a lot of interesting information and it helps me to continue thinking about good coding practices. I hope it helps you as well get started down the right path to clean code. Thanks!