SOLID is a very popular acronym in the software industry, and first letter S stands for a Single Responsibility Principle, a very powerful term and technique that helps fight software complexity. Everyone knows it, everyone talks about it, and yet, it's still hard to use properly.
By default, the principle is based on a class or a module responsibility. Methods could benefit also.
Live sample from the real world...
public class LoggerUtil
{
public static string GenerateLogContentWithRequestInfo(Exception ex, HttpRequest request)
{
try
{
request.Body.Seek(0, SeekOrigin.Begin);
string requestBodyString;
using (var reader = new StreamReader(request.Body))
{
requestBodyString = reader.ReadToEnd();
}
return $"{ ex } { requestBodyString } { request.Headers.ToJson() }";
}
catch (Exception ex1)
{
return $"Failed with: { ex1 } when processing: { ex }";
}
}
}
...and its usage.
_logger.LogError(LoggerUtil.GenerateLogContentWithRequestInfo(ex, Request));
The method tries to bundle the request and exception together to return them as a string for a logger to use it. Going through lines, it's clear that most of them are busy serializing the request, while the exception is used only on the last step when combined into a result. This should be the first sign of something being wrong.
Things are getting worse when we try to recover from an exception in a catch clause. At this point, there are two exceptions which will make it harder to analyze the log. And there won't be much help from the stack trace here.
The solution, however, is pretty simple. Make method responsible for one and one thing only; serializing a request. Something like this...
public static class RequestExtensions
{
public static string Stringify(this HttpRequest request)
{
try
{
request.Body.Seek(0, SeekOrigin.Begin);
string requestBodyString;
using (var reader = new StreamReader(request.Body))
{
requestBodyString = reader.ReadToEnd();
}
return $"{ requestBodyString } { request.Headers.ToJson() }";
}
catch (Exception ex)
{
return $"Failed with: { ex } while processing request";
}
}
}
As a nice addition, method could be converted to an extension one...
_logger.LogError($"{ ex } { httpContext.Request.Stringify() }");
This is easier to read and more flexible to use; the log message could be defined when necessary, instead of being specified somewhere very far from the context details.
Bonus
Shorter (minus 2 lines!) version of sample above. Less code is better!
public static class RequestExtensions
{
public static string Stringify(this HttpRequest request)
{
try
{
request.Body.Seek(0, SeekOrigin.Begin);
using (var reader = new StreamReader(request.Body))
{
return $"{ reader.ReadToEnd() } { request.Headers.ToJson() }";
}
}
catch (Exception ex)
{
return $"Failed with: { ex } while processing request";
}
}
}