Some developers like to write one line of code for complex tasks. It’s called code golf and there is a whole subdomain on StackExchange dedicated to code golf. Also, I have seen an idea mentioned on some forums that you should never have a method that is a single line of code. I am going to challenge that statement and suggest that when a single line of code is difficult to understand, wrapping it in a method for the sole purpose of readability is a good practice to follow.

Below is an example of one line of code.

for (int i = 0; i < max; i++) { wsProducts[i].Features = dbContext.Products.Select(p=>p.Id == wsProducts.Id).Features.Select(f=>f.ToServiceObj()).ToList(); }

I am not going to argue whether one line of code is good or bad to have on one line. I like my for loops to be broken out like this.

for (int i = 0; i < max; i++) 
{ 
  wsProducts[i].Features = dbContext.Products.Select(p=>p.Id == wsProducts.Id).Features.Select(f=>f.ToServiceObj()).ToList();
}

But I am not going to dictate my personal preference onto other developers. That is not the point of this article. The point of this article is to talk about the benefit of a method for the sole purpose of documentation and making the code more readable. Besides, there are hundreds of other single lines of code that are difficult to understand. Thanks to Linq alone, C# now has plenty of examples. But this isn’t just a C# concept. This concept work in C++, Java, JavaScript, or any language. This concept is language agnostic.

So to start with, what is the above code doing? Can you tell from this line of code? I couldn’t at first glance. I had to examine it further. Who wrote this. (Hopefully, it wasn’t me two years ago. It probably was.)

Well, my ORM has Products and each product has a list of Features. My WebService also has Products and each Product has a list of Features. However, the ORM Product and Feature classes are not the same object types as the WebService Product and Feature classes. They are different objects in different namespaces. So basically, this code gets the list of features foreach product from the database and converts the features to a WebService Feature type, puts them in a list and assigns them to the WebService Product type’s feature list.

Wait, why did I have to explain that to you. Why didn’t you simply know what the code did? Because the code is not self-explanatory. Is is not easy to read or understand.

What if instead of our embedding our loop in our current code, we created and called this method instead?

GetFeaturesFromDatabase(MyDbContext dbContext, IEnumarable<MyWebService.Product> wsProducts) 
{
  for (int i = 0; i < max; i++) { wsProducts[i].Features = dbContext.Products.Select(p=>p.Id == wsProducts.Id).Features.Select(f=>f.ToServiceObj()).ToList(); }
}

Basically, we encapsulate (did I just use the term encapsulation outside of a CS 101 course) the complex code in a method and use the method instead.

GetFeaturesFromDatabase(dbContext, wsProducts);

Is that not clearer and easier to read?

But should we do this?

Let’s assume that our code already uses dependency injection and we already can mock the dbContext, and our code already has Unit Tests that are passing. So we don’t really need this method for any other reason other than documentation.

My answer is YES! Yes, using a method for the sole purpose making the code self-documenting and easier to read is worth it.

What do you think?