Problem
I have a console application that basically retrieves XML content, writes to the XML file and sends the file to an SFTP site:
public Main(string[] args)
{
try
{
//code to parse arguments to load into DTO to extract stored proc and report name
List<ReportInfo> reports = Parse(args);
foreach(var rpt in reports)
{
//retrieve xml content from database
XmlDocument doc = new XmlDocument();
using (SqlConnection con = new SqlConnection(AppConfig.ConnectionString))
{
using (SqlCommand cmd = con.CreateCommand())
{
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = rpt.StoredProcedure;
con.Open();
using (var reader = cmd.ExecuteXmlReader())
{
doc.Load(reader);
reader.Close();
}
con.Close();
}
}
//save xml file in a folder
string filePath = Path.Combine(AppConfig.ReportsFolder, string.Format(rpt.FileName, DateTime.Today.ToString("MMddyyyy")));
using (var fileStream = new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None))
{
using (var xmlWriter = XmlWriter.Create(fileStream,
new XmlWriterSettings
{
OmitXmlDeclaration = false,
ConformanceLevel = ConformanceLevel.Document,
Encoding = Encoding.UTF8
}))
{
xmldoc.Save(xmlWriter);
}
}
// third party tool is called to transmit the file
SFtpClient client = new SFtpClient("host","user","pwd");
client.Send(filPpath);
}
}
catch(Exception ex)
{
_iLogger.Error(ex);
}
}
Main()
consists of considerable amount of lines. So, I have decided to split functionality into smaller classes similar to SOLID principles like this:
public Main(string[] args)
{
try
{
List<ReportInfo> reports = Parse(args);
foreach(var rpt in reports)
{
XmlDocument xmldoc = DBHelper.GetReport(rpt.sproc);
var filePath = ReportProcessor.SaveFileAsXml(xmldoc);
ReportProcessor.SendFileviaSFtp(filePath);
}
}
catch(Exception ex)
{
_iLogger.Error(ex);
}
}
public static class ParametersParser
{
public static List<ReportsInfo> Parse(string[] args)
{
//parse the args
}
}
public static class DBHelper
{
public static XmlDocument GetReport(string storedprocedure)
{
XmlDocument doc = new XmlDocument();
using (SqlConnection con = new SqlConnection(AppConfig.ConnectionString))
{
using (SqlCommand cmd = con.CreateCommand())
{
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = storedprocedure;
con.Open();
using (var reader = cmd.ExecuteXmlReader())
{
doc.Load(reader);
reader.Close();
}
con.Close();
}
}
return doc;
}
}
public static class ReportProcessor
{
public static string SaveFileAsXml(string fileName, XmlDocument xmldoc)
{
string filePath = Path.Combine(AppConfig.ReportsFolder, string.Format(fileName, DateTime.Today.ToString("MMddyyyy")));
using (var fileStream = new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None))
{
using (var xmlWriter = XmlWriter.Create(fileStream,
new XmlWriterSettings
{
OmitXmlDeclaration = false,
ConformanceLevel = ConformanceLevel.Document,
Encoding = Encoding.UTF8
}))
{
xmldoc.Save(xmlWriter);
}
}
return filePath;
}
public static void SendFileviaSFtp(string filePath)
{
//here third party tool is called to transmit the file
SFtpClient client = new SFtpClient("host","user","pwd");
client.Send(filepath);
}
}
Here are my doubts with these modifications:
- As observed, the classes are static. I felt they are more of utility classes and placed them all in
Helpers
folder. So, is it advisable to use static classes this way or shall I shift to normal classes? - By having static classes and methods, do I run into threading issues as there might be chances multiple instances of this exe might execute concurrently?
-
Even after splitting into different classes, I have the exception handling and logging inside
Main()
. Is it ok to have it handle at single location or shall I also include in individual methods and throw specific exception?public static class ParametersParser { public static List<ReportsInfo> Parse(string[] args) { try { //parse the args } catch(Exception ex) { throw new Exception("ParametersParser.Parse", ex); } } }
Solution
I would start by looking at functional decomposition. Here’s crack #1 at it:
internal static class Solid
{
private static readonly XmlWriterSettings settings = new XmlWriterSettings
{
OmitXmlDeclaration = false,
ConformanceLevel = ConformanceLevel.Document,
Encoding = Encoding.UTF8
};
public static void Main(string[] args)
{
try
{
ProcessReports(args, AppConfig.ConnectionString, AppConfig.ReportsFolder);
}
catch (Exception ex)
{
_iLogger.Error(ex);
}
}
private static void ProcessReports(string[] args, string connectionString, string reportsFolder)
{
// code to parse arguments to load into DTO to extract stored proc and report name
foreach (var rpt in Parse(args))
{
ProcessSingleReport(connectionString, reportsFolder, rpt);
}
}
private static IEnumerable<ReportInfo> Parse(string[] args)
{
// Do actual argument parsing here...
return Enumerable.Empty<ReportInfo>();
}
private static void ProcessSingleReport(string connectionString, string reportsFolder, ReportInfo rpt)
{
var doc = LoadReportXml(connectionString, rpt);
var filePath = Path.Combine(reportsFolder, string.Format(rpt.FileName, DateTime.Today.ToString("MMddyyyy")));
WriteFile(filePath, doc);
TransmitFile(filePath);
}
private static XmlDocument LoadReportXml(string connectionString, ReportInfo rpt)
{
using (var con = new SqlConnection(connectionString))
using (var cmd = con.CreateCommand())
{
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = rpt.StoredProcedure;
con.Open();
using (var reader = cmd.ExecuteXmlReader())
{
var doc = new XmlDocument();
doc.Load(reader);
return doc;
}
}
}
private static void WriteFile(string filePath, XmlDocument doc)
{
using (var fileStream = new FileStream(filePath, FileMode.Create, FileAccess.Write, FileShare.None))
using (var xmlWriter = XmlWriter.Create(fileStream, settings))
{
doc.Save(xmlWriter);
}
}
private static void TransmitFile(string filePath)
{
// third party tool is called to transmit the file
SFtpClient client = new SFtpClient("host", "user", "pwd");
client.Send(filePath);
}
}
Then, you can see there’s a definite separation of concerns, and therefore separate classes:
internal static class Solid
{
public static void Main(string[] args)
{
try
{
var argumentParser = new ArgumentParser(args);
var reportLoader = new ReportLoader(AppConfig.ConnectionString);
var reportProcessor = new ReportProcessor(AppConfig.ReportsFolder, reportLoader);
var reportsProcessor = new ReportsProcessor(argumentParser, reportProcessor);
reportsProcessor.ProcessReports();
}
catch (Exception ex)
{
_iLogger.Error(ex);
}
}
}
/// <summary>
/// code to parse arguments to load into DTO to extract stored proc and report name
/// </summary>
internal sealed class ArgumentParser
{
private readonly string[] args;
public ArgumentParser(string[] args)
{
this.args = args;
}
public IEnumerable<ReportInfo> Parse()
{
// Do actual argument parsing here...
return Enumerable.Empty<ReportInfo>();
}
}
internal sealed class ReportLoader
{
private readonly string connectionString;
public ReportLoader(string connectionString)
{
this.connectionString = connectionString;
}
public XmlDocument LoadReportXml(ReportInfo rpt)
{
using (var con = new SqlConnection(this.connectionString))
using (var cmd = con.CreateCommand())
{
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = rpt.StoredProcedure;
con.Open();
using (var reader = cmd.ExecuteXmlReader())
{
var doc = new XmlDocument();
doc.Load(reader);
return doc;
}
}
}
}
internal sealed class ReportProcessor
{
private readonly string reportsFolder;
private readonly ReportLoader reportLoader;
public ReportProcessor(string reportsFolder, ReportLoader reportLoader)
{
this.reportsFolder = reportsFolder;
this.reportLoader = reportLoader;
}
public void ProcessSingleReport(ReportInfo rpt)
{
var doc = this.reportLoader.LoadReportXml(rpt);
var filePath = Path.Combine(this.reportsFolder, string.Format(rpt.FileName, DateTime.Today.ToString("MMddyyyy")));
new ReportWriter(filePath, doc).WriteFile();
new Transmitter(filePath).TransmitFile();
}
}
internal sealed class ReportsProcessor
{
private readonly ArgumentParser parser;
private readonly ReportProcessor reportProcessor;
public ReportsProcessor(ArgumentParser parser, ReportProcessor reportProcessor)
{
this.parser = parser;
this.reportProcessor = reportProcessor;
}
public void ProcessReports()
{
foreach (var rpt in this.parser.Parse())
{
this.reportProcessor.ProcessSingleReport(rpt);
}
}
}
internal sealed class ReportWriter
{
private static readonly XmlWriterSettings settings = new XmlWriterSettings
{
OmitXmlDeclaration = false,
ConformanceLevel = ConformanceLevel.Document,
Encoding = Encoding.UTF8
};
private readonly string filePath;
private readonly XmlDocument doc;
public ReportWriter(string filePath, XmlDocument doc)
{
this.filePath = filePath;
this.doc = doc;
}
public void WriteFile()
{
using (var fileStream = new FileStream(this.filePath, FileMode.Create, FileAccess.Write, FileShare.None))
using (var xmlWriter = XmlWriter.Create(fileStream, settings))
{
this.doc.Save(xmlWriter);
}
}
}
internal sealed class Transmitter
{
private readonly string filePath;
public Transmitter(string filePath)
{
this.filePath = filePath;
}
public void TransmitFile()
{
// third party tool is called to transmit the file
SFtpClient client = new SFtpClient("host", "user", "pwd");
client.Send(this.filePath);
}
}
Extract out some interface
s for each of these classes, inject some more of the smaller dependencies here and there, and you’ll have a pretty SOLID class structure.