söndag 26 november 2017

Refactoring: Replace Magic Number with Symbolic Constant example i C#

Part three in my series of refactors I've used the most from the book Refactoring - Improving the design of existing code.

Replace Magic Number with Symbolic Constant

You have a literal number with a particular meaning.
Create a constant, name it after the meaning, and replace the number with it.

Example 1

The code before

1:  double PotentialEnergy(double mass, double height)  
2:  {  
3:    return mass * 9.81 * height;  
4:  }  

The code after

1:  private const double GravitationalConstant = 9.81;  
3:  double PotentialEnergy(double mass, double height)  
4:  {  
5:    return mass * GravitationalConstant * height;  
6:  }  

Example 2

I thought I would find a better example, but it wasn't that easy, so I settled with just another example. The example below is from the page https://www.eliotsykes.com/magic-numbers, but I think the real world examples in the bottom of that page might give you a better understanding of how you can use constants.

The code before

1:  public bool AllowedToComment()  
2:  {  
3:    return _age >= 13 && CommentsInLastHour.Count < 20;  
4:  }  

The code after

1:  const int CommenterMinAge = 13;  
2:  const int MaxCommentsPerHour = 20;  
4:  public bool AllowedToComment()  
5:  {  
6:    return _age >= CommenterMinAge && CommentsInLastHour.Count < MaxCommentsPerHour;  
7:  }  


Magic numbers are one of the oldest ills in computing. They are numbers with special values that usually are not obvious. Magic numbers are really nasty when you need to reference the same logical number in more than one place. If the numbers might ever change, making the change is a nightmare. Even if you don't make a change, you have the difficulty of figuring out what is going on.

The source code

måndag 13 november 2017

Refactoring: Introduce Explaining Variable example i C#

Part two in my series of refactors I've used the most from the book Refactoring - Improving the design of existing code.

Introduce Explaining Variable

More known as Extract Variable

You have a complicated expression.
Put the result of the expression, or parts of the expression, in a temporary variable with a name that explains the purpose.

Example 1: The code before

1:  if (platform.ToUpper().IndexOf("MAC") > -1 &&  
2:    browser.ToUpper().IndexOf("IE") > -1 &&  
3:    WasInitialized() && resize > 0)  
4:  {  
5:    // do something  
6:  }  

Example 1: The code after

1:  bool isMacOs = platform.ToUpper().IndexOf("MAC") > -1;  
2:  bool isIEBrowser = browser.ToUpper().IndexOf("IE") > -1;  
3:  bool wasResized = resize > 0;  
5:  if (isMacOs && isIEBrowser && WasInitialized() && wasResized)  
6:  {  
7:    // do something  
8:  }  

Example 2: The code before

1:  public double Price()  
2:  {  
3:    // price is base price - quantity discount + shipping  
4:    return _quantity * _itemPrice -  
5:        Math.Max(0, _quantity - 500) * _itemPrice * 0.05 +  
6:        Math.Min(_quantity * _itemPrice * 0.1, 100);  
7:  }  

Example 2: The code after

1:  public double Price()  
2:  {  
3:    var basePrice = _quantity * _itemPrice;  
4:    var quantityDiscount = Math.Max(0, _quantity - 500) * _itemPrice * 0.05;  
5:    var shipping = Math.Min(_quantity * _itemPrice * 0.1, 100);  
6:    return basePrice - quantityDiscount + shipping;  
7:  }  

Example 2: The code after (using Extract Method)

1:  public double Price()  
2:  {  
3:    return BasePrice() - QuantityDiscount() + Shipping();  
4:  }  
6:  private int BasePrice()  
7:  {  
8:    return _quantity * _itemPrice;  
9:  }  
11:  private double QuantityDiscount()  
12:  {  
13:    return Math.Max(0, _quantity - 500) * _itemPrice * 0.05;  
14:  }  
16:  private double Shipping()  
17:  {  
18:    return Math.Min(_quantity * _itemPrice * 0.1, 100);  
19:  }  

Example 2: The code after (using Expression Body Definitions)

If you're using C# 6 or above you can use Expression Body Definitions to shorten your methods.

1:  public double Price()  
2:    => BasePrice - QuantityDiscount + Shipping;  
4:  private int BasePrice   
5:    => _quantity * _itemPrice;  
7:  private double QuantityDiscount   
8:    => Math.Max(0, _quantity - 500) * _itemPrice * 0.05;  
10:  private double Shipping  
11:    => Math.Min(_quantity * _itemPrice * 0.1, 100);  


Expressions can become very complex and hard to read. In such situations temporary variables can be helpful to break down the expression into something more manageable.
Introduce Explaining Variable is particularly valuable with conditional logic in which it is useful to take each clause of a condition and explain what the condition means with a well-named temp.

The source code

Personal thoughts

An easy-to-do refactoring that can do so much for the code readability. Low hanging fruit!

söndag 12 november 2017

Refactoring: Extract Method example in C#

Many years ago I read the book Refactoring - Improving the design of existing code by Martin Fowler and found that it contained lots of interesting thoughts about code readability.

Instead of just doing a reread of it, I thought it might be interesting to write about the refactors that I've had the most use of during my time as a (mostly web) developer.

The examples in the book are written i Java, I will copy and translate them to C#.

I begin the series with the Extract Method refactoring.

Extract Method

You have a code fragment that can be grouped together.
Turn the fragment into a method whose name explains the purpose of the method.

The code before

The example is a short method that prints out how much a customer owes.

1:  public void PrintOwing()  
2:  {  
3:    Decimal outstanding = 0;  
5:    // print banner  
6:    Console.Out.WriteLine("*************************");  
7:    Console.Out.WriteLine("***** Customer Owes *****");  
8:    Console.Out.WriteLine("*************************");  
10:    // calculate outstanding  
11:    foreach (var order in _orders)  
12:    {  
13:      outstanding += order.GetAmount();  
14:    }  
16:    // print details  
17:    Console.Out.WriteLine("name: " + _name);  
18:    Console.Out.WriteLine("amount: " + outstanding);  
19:  }  

The code after

The three sections in PrintOwing has been moved out to three separate methods, which makes the PrintOwing method really short and you get the big picture of what it does real easy.

1:  public void PrintOwing()  
2:  {  
3:    PrintBanner();  
4:    var outstanding = GetOutstanding();  
5:    PrintDetails(outstanding);  
6:  }  
8:  private void PrintBanner()  
9:  {  
10:    Console.Out.WriteLine("*************************");  
11:    Console.Out.WriteLine("***** Customer Owes *****");  
12:    Console.Out.WriteLine("*************************");  
13:  }  
15:  private decimal GetOutstanding()  
16:  {  
17:    Decimal outstanding = 0;  
18:    foreach (var order in _orders)  
19:    {  
20:      outstanding += order.GetAmount();  
21:    }  
22:    return outstanding;  
23:  }  
25:  private void PrintDetails(decimal outstanding)  
26:  {  
27:    Console.Out.WriteLine("name: " + _name);  
28:    Console.Out.WriteLine("amount: " + outstanding);  
29:  }  


The big wins with the extract method refactoring is that the code is divided into fine grained chunks that increases the chance of code reuse, and that the code itself can be read more like a series of comments.

How to Refactor

Read more about how to do this refactor step-by-step and more here.

The source code

Personal thoughts

Replace foreach with Linq

I really like to Linq-ify code when it makes the code shorter and better expresses what it does. Like replacing a foreach loop with Linqs Sum method.

I prefer this
1:  private decimal GetOutstanding()  
2:  {  
3:    return _orders.Sum(order => order.GetAmount());  
4:  }  

instead of this
1:  private decimal GetOutstanding()  
2:  {  
3:    Decimal outstanding = 0;  
4:    foreach (var order in _orders)  
5:    {  
6:      outstanding += order.GetAmount();  
7:    }  
8:    return outstanding;  
9:  }  

because I think Sum shows more clearly than the foreach loop what we're after.

Thoughts that popped up during writing

The PrintOwing method was short to begin with. Did the three extract method operations really make it better? I've performed the extract method on much longer methods, where I clearly saw the advantage, but here I don't find it quite as clear.

One thing that happens when you extract code to smaller methods is that its makes the code easier to reuse. Suppose I want to print the same banner from another method, after the refactoring I can just call the PrintBanner method instead of copy-pasting the code.

So, what's the problem with copy-paste coding? Well, suppose you need to print the same banner in five different methods. If you haven't extracted the banner printing code to a separate method the same code will exist at five places. What happens when your customer wants you to change the banner layout, maybe change the "*" to a "#"? Will you find every copy of the banner printing code? With five different copies of it, I think the risk is high that some of the copies will be forgotten to be changed, and voila, you've created a bug!

My guess is that although PrintOwing was short from the start, it is possible to understand the big picture about what it does faster when it is even shorter. Compare these two really hypothetical eye movement scanning examples when reading the code to get the big picture of what PrintOwing does.

In the example above, you have to read a lot of code to understand the big picture of what the code does. That's bad, but on the other side, you get a good bit of insight into the details. You can see that the eyes make big jumps up and down when looking for a variable and what value it has been assigned to, and how it has been used. Details are mixed with the bigger picture.

In this second example the eyes have to scan a lot less code and don't have to do big jumps to find out where the variable gets its value from.

Maybe you would scan the code in the first example a lot different than I've drawn. Maybe you scan for comments, read them and skip the code. Well, I read comments as well, but my experience is that way too often the comment related to the code does not really match the code. The comment says one thing, and the code does something like it but not quite, or maybe even something waaay off.
In this example, the comments are replaced with the names of the methods the code is extracted to, so the problem can still exist, if the method is given a bad name it won't match the code it contains.