One of the principles I introduce in my new talk is Strive for Functional Cohesion. My original article on the subject focuses on determining where the functionality belongs, then adding it to it’s appropriate place. In that scenario, I had complete access to the code. Sometimes, however, you don’t have access to the code to make the change, or the implementation belongs to an interface. If the functionality is assign appropriately, it makes the code much easier to read.
var lines = File.ReadAllLines("file.txt");
var reversed = lines.Select(s => String.Join(",", s.Split(',').Reverse().ToArray())).ToArray();
File.WriteAllLines("file.txt", reversed);
Although procedural, it is declarative. We’re reading lines, reversing them, then writing them back out. We’ve avoided the morass that is iterative programming here by using LINQ. Despite the benefit from being declarative, it isn’t very readable. In my presentation, I show how by assigning the functionality to the class or interface it belongs makes the code more readable. In this case, an IEnumberable<string> should be able to delimit itself, and it should be able to write itself out.
File.ReadAllLines("file.txt")
.Select(s => s.Split(',').Reverse().Delimit())
.Write("file.txt");
This cleanup was accomplished with two, very reusable, extension methods (there’s an overload for Delimit in case it isn’t comma-separated). These extensions are available in Fluent.NET.
public static class EnumerableStringExtensions
{
private const string Delimiter = ",";
public static string Delimit(this IEnumerable<string> strings)
{
return strings.Delimit(Delimiter);
}
public static string Delimit(this IEnumerable<string> strings, string delimiter)
{
return string.Join(delimiter, strings.ToArray());
}
public static void Write(this IEnumerable<string> strings, string path)
{
File.WriteAllLines(path, strings.ToArray());
}
}
While the application of principles enabled me to write cleaner code, it struck me while I was giving this presentation that I still had to explain what how the reversal occurred. If I have to explain it to an audience, someone is probably having to spend too many brain cycles reading my code to figure out what’s going on. It is easier to figure out that s.Split(‘,’).Reverse().Delimit() reverses a set of delimited strings than String.Join(",", l.Split(',').Reverse().ToArray())).ToArray(), but there must be a way to make the code even easier to read.
Reduce Chain
The first way the statement can be simplified is by using the Reduce Chain refactoring. This takes common links from a method chain and reduces it to one link. In this case, we have 3 methods forming our chain: Split, Reverse, and Delimit. We are operating on the string class, so an extension method will be necessary. Since is a delimited string being reversed, I will call the method DelimitedReverse().
public static class StringExtensions
{
public static string DelimitedReverse(this string source)
{
return source.Split(',').Reverse().Delimit();
}
}
Any place this common method chain was being called can be reduced to this statement. This adheres to the DRY principle by cleaning up those repeated chains.
File.ReadAllLines("file.txt")
.Select(s => s.DelimitedReverse())
.Write("file.txt");
This refactoring is useful if the chain is being called from many different places, but I suspect that I will be making this call only in the context of a LINQ select. There’s another refactoring for that.
Extract Projection
Projection is the term used to describe transforming one sequence of items to another. The LINQ Select statement is the de facto way in C# to perform a projection. In the example, we’re doing a DelimitedReverse() projection on IEnumerable<string>. If we extract the projection and give it a name, the code will be easier to read. If that projection was used in more than one place, then the duplication has been removed as well.
There are two ways to do this. The first way merely passes through to a LINQ Select.
public static IEnumerable<string> DelimitedReverse(this IEnumerable<string> strings)
{
return strings.Select(s => s.Split(',').Reverse().Delimit());
}
The second way builds an iterator using the yield keyword.
public static IEnumerable<string> DelimitedReverse(this IEnumerable<string> strings)
{
foreach (var s in strings)
{
yield return s.Split(',').Reverse().Delimit();
}
}
My opinion on which is better? Go with the declarative, pass through version unless the projection is nasty. If your code is easier to read in the iterative version, do it that way.
Here’s how the code now reads.
File.ReadAllLines("file.txt").DelimitedReverse().Write("file.txt");
It’s much better than the original. It’s even easier to read than the version refactored for functional cohesion.
Subscribe