I work as a C# developer at a company that doesn't use best practices at all. We're on .NET 3.5 but most code is written in a .NET 1.1 style (e.g. almost all the logic is in the code behind of the ASPX page, everything uses untyped DataSets instead of returning objects, makes gratuitous use of Session and QueryString to pass data, very little architectural patterns). I have spent a couple of years learning best practices by reading blogs and watching screencasts from the top echelon of .Net developers.
I was recently tasked with creating a new feature on our in-house ERP system that writes data from XML to a PDF for display. It was suggested that we might want various types of PDFs in the future, so the framework should be flexible. I took the opportunity to apply proper design patterns and a DDD-like approach to the code, with the idea being to demonstrate a "better way" of doing things to my peers. My code was subsequently rejected in a "code review" by the lead developer/development manager as being "too complicated" compared to simply writing all the logic in a single class or in the code-behind (what my other co-workers would have done had they been given the project instead). I managed to get the code passed anyways as they didn't want me to "waste time" going back and changing it (they consider refactoring a waste of time that adds no business value), but it got me thinking if the code really is too complex; I appear to be following all of the best practices, I am following (I think) all of the SOLID principles, I am using proper design patterns and OOP software engineering techniques.
Here is a simplified version of the classes (may or may not be 100% compilable as it's a stripped down version; you should get the idea though):
// Repositories
public interface IRepository { }
public interface IXmlRepository : IRepository {
XmlDocument GetXml();
}
// Repositories.Impl
public abstract class BillingXmlRepository : IXmlRepository {
protected long quoteID;
public BillingXmlRepository(long quoteID) {
this.quoteID = quoteID;
}
public abstract XmlDocument GetXml();
}
public sealed class XmlInvoiceRepository : BillingXmlRepository {
public XmlInvoiceRepository(long quoteID) : base(quoteID) { }
public override XmlDocument GetXml() {
// XML retrival here...
}
}
// DataAggregation
pubilc interface IAggregatableData<T> {
T GetData();
}
public abstract class DataAggregator<T> : IAggregatableData<T> {
public abstract T GetData();
protected abstract ICollection<IRepository> GetAllRequiredRepositories();
}
public abstract class XmlDataAggregator : DataAggregator<XmlDocument> {
public override XmlDocument GetData() {
XmlDocument root = new XmlDocument();
foreach (IRepository repository in this.GetAllRequiredRepositories()) {
XmlDocument xml = (repository as IXmlRepository).GetXml();
XmlDocumentFragment fragment = root.CreateDocumentFragment();
fragment.InnerXml = xml.InnerXml;
root.DocumentElement.AppendChild(fragment);
}
return root;
}
}
public sealed class BillingDataAggregator : XmlDataAggregator {
private long quoteID;
public BillingDataAggregator(long quoteID) {
this.quoteID = quoteID;
}
protected override void ICollection<IRepository> GetAllRequiredRepositories() {
return new List<IRepository> {
new XmlBillingRepository(this.quoteID);
}
}
}
// Mappers to map raw XML to classes
public interface IMappable<T, K> {
T Map(K rawData);
}
public interface IXmlMappable<T> : IMappable<T, XmlDocument> {
}
public sealed class BillingXmlMapper : IXmlMappable<BillingInfo> {
public BillingInfo Map(XmlDocument rawData) {
// LINQ code to traverse the XML and map it into a BillingInfo DTO
}
}
// Entities
public sealed class BillingInfo {
// simple properties
}
// Consumer example
XmlDataAggregator aggregator = new BillingDataAggregator(1234);
var xml = aggregator.GetData();
var mapper = new BillingXmlMapper();
BillingInfo info = mapper.Map(xml);
Am I on the right track as far as following the best way to write modular code? I can't bring myself to ignore good practices and write all the code in code-behind files or the like.