CodeSOD: A Winning Strategy

"Hey," Roberto said while pairing with an offshore programmer, "this problem would be easier to solve with the Factory pattern."

"What's that?"

Roberto explained both the Factory pattern and the idea of design patterns, and congratulated himself on helping a fellow developer improve their skills. Little did he know, he had created a monster.

Things started cropping up in his code base. For example, Roberto found this block:

var SetInfoAsDateList = SetInfoList.Where(c => c.ValueType.Contains("Date")); foreach (var item in SetInfoAsDateList) { //Use the Strategy pattern to find the correct date object. dateProcessor = GetDateProcessor(item.LabelName); dateProcessor.ProcessDate(item, SetInfoList, criteria, agentInformation, agentOptions); }

Oh, the Strategy pattern? Let's go take a look at how GetDateProcessor was implemented.

private static IDateProcessor GetDateProcessor(string value){ IDateProcessor dateStrategy = null; switch (value) { case "Change Date": dateStrategy = new DateFormatProcessor(); break; case "Contract Date": dateStrategy = new DateFormatProcessor(); break; case "List Date": dateStrategy = new DateFormatProcessor(); break; case "ListingUpdateType": dateStrategy = new DateFormatProcessor(); break; //case "Mobile Home Mfr. Date": // dateStrategy = new DateFormatProcessor(); // break; case "Sold Date": dateStrategy = new DateFormatProcessor(); break; case "Withdrawn Date": dateStrategy = new DateFormatProcessor(); break; case "Available Date": dateStrategy = new DateFormatProcessor(); break; default: break; } return dateStrategy;}

Oh, like that. It's" extensible, at least, I suppose. At the risk of seeing yet another "I wrote my own date processing engine" WTF, should we take a look at what the DateFormatProcessor.ProcessDate function does?

public class DateFormatProcessor : IDateProcessor{ public void ProcessDate(result DetailSet, List<result> items, StatsCriteria criteria, login agentInformation, agent_options agentOptions) { var AvailableDate = items.Where(i => i.LabelName == "Available Date" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); var ChangeDate = items.Where(i => i.LabelName == "Change Date" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); var ContractDate = items.Where(i => i.LabelName == "Contract Date" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); var ListDate = items.Where(i => i.LabelName == "List Date" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); var ListingUpdateType = items.Where(i => i.LabelName == "ListingUpdateType" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); var MobileHomeMfrDate = items.Where(i => i.LabelName == "Mobile Home Mfr. Date" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); var SoldDate = items.Where(i => i.LabelName == "Sold Date" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); var WithdrawnDate = items.Where(i => i.LabelName == "Withdrawn Date" && i.MlsNum == DetailSet.MlsNum && i.ColumnHeader != "META").FirstOrDefault(); if (DetailSet.LabelName == "Withdrawn Date") { if (!String.IsNullOrEmpty(WithdrawnDate.LabelValue)) { DetailSet.LabelValue = String.IsNullOrEmpty(DetailSet.LabelValue) ? "" : String.Format("{0:MM/dd/yyyy}", Convert.ToDateTime(DetailSet.LabelValue)); DetailSet.LabelValue = String.Format("{0:MM/dd/yyyy}", DetailSet.LabelValue); } else { DetailSet.LabelValue = ""; } } if (DetailSet.LabelName == "Sold Date") { if (!String.IsNullOrEmpty(SoldDate.LabelValue)) { DetailSet.LabelValue = String.IsNullOrEmpty(DetailSet.LabelValue) ? "" : String.Format("{0:MM/dd/yyyy}", Convert.ToDateTime(DetailSet.LabelValue)); DetailSet.LabelValue = String.Format("{0:MM/dd/yyyy}", DetailSet.LabelValue); } else { DetailSet.LabelValue = ""; } } if (DetailSet.LabelName == "List Date") { if (!String.IsNullOrEmpty(ListDate.LabelValue)) { DetailSet.LabelValue = String.IsNullOrEmpty(DetailSet.LabelValue) ? "" : String.Format("{0:MM/dd/yyyy}", Convert.ToDateTime(DetailSet.LabelValue)); DetailSet.LabelValue = String.Format("{0:MM/dd/yyyy}", DetailSet.LabelValue); } else { DetailSet.LabelValue = ""; } } if (DetailSet.LabelName == "Contract Date") { if (!String.IsNullOrEmpty(ContractDate.LabelValue)) { DetailSet.LabelValue = String.IsNullOrEmpty(DetailSet.LabelValue) ? "" : String.Format("{0:MM/dd/yyyy}", Convert.ToDateTime(DetailSet.LabelValue)); DetailSet.LabelValue = String.Format("{0:MM/dd/yyyy}", DetailSet.LabelValue); } else { DetailSet.LabelValue = ""; } } if (DetailSet.LabelName == "Change Date") { if (!String.IsNullOrEmpty(ChangeDate.LabelValue)) { DetailSet.LabelValue = String.IsNullOrEmpty(DetailSet.LabelValue) ? "" : String.Format("{0:MM/dd/yyyy}", Convert.ToDateTime(DetailSet.LabelValue)); DetailSet.LabelValue = String.Format("{0:MM/dd/yyyy}", DetailSet.LabelValue); } else { DetailSet.LabelValue = ""; } } if (DetailSet.LabelName == "Available Date") { if (!String.IsNullOrEmpty(AvailableDate.LabelValue)) { DetailSet.LabelValue = String.IsNullOrEmpty(DetailSet.LabelValue) ? "" : String.Format("{0:MM/dd/yyyy}", Convert.ToDateTime(DetailSet.LabelValue)); DetailSet.LabelValue = String.Format("{0:MM/dd/yyyy}", DetailSet.LabelValue); } else { DetailSet.LabelValue = ""; } } }}

Maybe he should have used a few more design patterns"

[Advertisement] Manage IT infrastructure as code across all environments with Puppet. Puppet Enterprise now offers more control and insight, with role-based access control, activity logging and all-new Puppet Apps. Start your free trial today!