Introduction
Please first refer to the original-article (and note it's exorbitant rating) - before examining my enhancements of it.
I tried to annoy the originals author with my statement: "Your solution is not a good example for easy-understand-coding".
His clever reply was, to invite me to present a 'better version' - from my point of view.
Ok - I'll try.
The Application
There is assumed some Data, which is to query in several ways, and the query-results - short summaries - are to display on the console.
See yourself
The original codes contained two versions - a perfect template to me, to add my third version.
To make it simple: I offer Version2 and Version3 to you - as they are. And you can inspect the listings and decide for yourself, which version you find easier to understand.
Please note: I show the file-contents as they are, without omissions, and especially without any additional explainations.
This articles topic is not to explain code - it is about making it as self-explainatory as possible.
In real-life there are also no additional explainations, and you must get by with the code as it is.
You propably will be right, when you find technical and architectural drawbacks - in several respects.
But as said: The topic is not "What is done?", but it is: "Can you understand, what is done?"
My version
File Datamodel.cs:
1
4 using System.ComponentModel;
5
6 namespace SimpleReport.Version3 {
7
8 public enum ReportType {
9 [Description("Company vs price")]
10 CompanyVsPrice = 1,
11 [Description("Company vs city")]
12 CompanyVsCity,
13 [Description("Company vs number of days")]
14 CityVsNumberOfDays,
15 [Description("Company vs number of person")]
16 CompanyVsNumberOfPerson
17 }
18
19 public class CompanyInfo {
21 public Months Month { get; set; }
22 public string CompanyName { get; set; }
23 public string City { get; set; }
24 public int NumberOfPerson { get; set; }
25 public int NumberOfDays { get; set; }
26 public decimal Price { get; set; }
27 }
28 }
File Report.cs:
29
30 using System;
31 using System.Collections.Generic;
32
33 namespace SimpleReport.Version3 {
34
35 public class Report {
37 public string Name { get; set; }
38 public List<ReportEntry> Entries { get; set; }
39 }
40
41 public class ReportEntry {
42 public string Caption { get; set; }
43 public string Value { get; set; }
44 }
45 }
File Repository.cs:
46
48 using System;
49 using System.Collections.Generic;
50 using System.Linq;
51
52 namespace SimpleReport.Version3 {
53 class Repository {
54
55
56 private static List<CompanyInfo> _CompanyInfos = new List<CompanyInfo>
57 {
58 new CompanyInfo { City = "City 1", CompanyName = "Company 1", Month = Months.January, NumberOfDays = 5, NumberOfPerson = 30, Price = 23045 },
59 new CompanyInfo { City = "City 1", CompanyName = "Company 1", Month = Months.February, NumberOfDays = 4, NumberOfPerson = 50, Price = 10000 },
60 new CompanyInfo { City = "City 1", CompanyName = "Company 1", Month = Months.March, NumberOfDays = 8, NumberOfPerson = 70, Price = 55000 },
61 new CompanyInfo { City = "City 2", CompanyName = "Company 1", Month = Months.April, NumberOfDays = 3, NumberOfPerson = 3, Price = 12000 },
62 new CompanyInfo { City = "City 2", CompanyName = "Company 2", Month = Months.January, NumberOfDays = 2, NumberOfPerson = 12, Price = 15000 },
63 new CompanyInfo { City = "City 2", CompanyName = "Company 2", Month = Months.June, NumberOfDays = 2, NumberOfPerson = 11, Price = 16000 },
64 new CompanyInfo { City = "City 3", CompanyName = "Company 1", Month = Months.July, NumberOfDays = 8, NumberOfPerson = 20, Price = 33000 },
65 new CompanyInfo { City = "City 3", CompanyName = "Company 3", Month = Months.August, NumberOfDays = 5, NumberOfPerson = 10, Price = 9000 },
66 new CompanyInfo { City = "City 1", CompanyName = "Company 1", Month = Months.September, NumberOfDays = 3, NumberOfPerson = 20, Price = 22000 },
67 new CompanyInfo { City = "City 1", CompanyName = "Company 1", Month = Months.October, NumberOfDays = 8, NumberOfPerson = 30, Price = 55000 },
68 };
69
70 public static Report GetCompanyInfoReport(ReportType reportType, Months month) {
71 var rep = new Report() { Name = Util.GetEnumDescription(reportType) };
72 var monthData = _CompanyInfos.Where(x => x.Month == month);
73 IEnumerable<ReportEntry> itms = null;
74 switch (reportType) {
75 case ReportType.CompanyVsPrice:
76 itms = from inf in monthData group inf by inf.CompanyName into g
77 select new ReportEntry { Caption = g.Key, Value = g.Sum(i => i.Price).ToString("F") };
78 break;
79 case ReportType.CompanyVsCity:
80 itms = from inf in monthData group inf by inf.CompanyName into g
81 select new ReportEntry { Caption = g.Key, Value = string.Join("; ", g.Select(i => i.City)) };
82 break;
83 case ReportType.CityVsNumberOfDays:
84 itms = from inf in monthData group inf by inf.City into g
85 select new ReportEntry { Caption = g.Key, Value = g.Sum(i => i.NumberOfDays).ToString() };
86 break;
87 case ReportType.CompanyVsNumberOfPerson:
88 itms = from inf in monthData group inf by inf.CompanyName into g
89 select new ReportEntry { Caption = g.Key, Value = g.Sum(i => i.NumberOfPerson).ToString() };
90 break;
91 default:
92 throw new ArgumentException("unknown reportType");
93 }
94 rep.Entries = itms.ToList();
95 return rep;
96 }
97 }
98 }
File Util.cs:
99
100 using System;
101 using System.Collections.Generic;
102 using System.ComponentModel;
103 using System.Linq;
104 using System.Reflection;
105
106 namespace SimpleReport.Version3 {
107
108 public enum Months { January = 1, February, March, April, May, June, July, August, September, October, November, December }
109
110 static class Util {
111
112 public static string GetEnumDescription(Enum value) {
114 var fi = value.GetType().GetField(value.ToString());
115 var attr = ((DescriptionAttribute[])fi.GetCustomAttributes(typeof(DescriptionAttribute), false));
116 return attr.Length == 0 ? value.ToString() : attr[0].Description;
117 }
118 }
119 }
The Original-Version
File BaseReportDefinition.cs:
1 using System;
2 using System.Collections.Generic;
3 using System.Linq;
4
5 namespace SimpleReport.Version2 {
6 internal abstract class BaseReportDefinition {
7 public virtual Func<Row, string> Aggregate { get { return reportItem => reportItem.CompanyName; } }
8
9 protected IEnumerable<IGrouping<string, Row>> GetReportBaseConditions(IList<Row> dataForReport, Month month) {
10 return dataForReport.Where(reportItem => reportItem.Month == (int)month).GroupBy(Aggregate);
11 }
12
13 protected string GetPriceAsString(decimal price) {
14 return price.ToString("F");
15 }
16
17 protected string GetAsString(IList<string> strings) {
18 return string.Join("; ", strings);
19 }
20 }
21 }
File CityVsNumberOfDaysReportDefinition.cs:
22 using System;
23 using System.Collections.Generic;
24 using System.Linq;
25
26 namespace SimpleReport.Version2 {
27
28 internal class CityVsNumberOfDaysReportDefinition : BaseReportDefinition, IReportDefinition {
29 public string ReportName { get { return "City vs number of days"; } }
30
31 public override Func<Row, string> Aggregate { get { return reportItem => reportItem.City; } }
32
33 public IList<ReportItem> GetReportItems(IList<Row> dataForReport, Month month) {
34 var reportItems = base.GetReportBaseConditions(dataForReport, month)
35 .Select(reportItem => new ReportItem {
36 ValueName = reportItem.Key,
37 Value = reportItem.Sum(reportValue => reportValue.NumberOfDays).ToString()
38 }).ToList();
39
40 return reportItems;
41 }
42 }
43 }
File CompanyVsCityReportDefinition.cs:
44 using System.Collections.Generic;
45 using System.Linq;
46
47 namespace SimpleReport.Version2 {
48 internal class CompanyVsCityReportDefinition : BaseReportDefinition, IReportDefinition {
49 public string ReportName { get { return "Company vs city"; } }
50
51 public IList<ReportItem> GetReportItems(IList<Row> dataForReport, Month month) {
52 var reportItems = base.GetReportBaseConditions(dataForReport, month)
53 .Select(reportItem => new ReportItem {
54 ValueName = reportItem.Key,
55 Value = base.GetAsString(reportItem.Select(reportValue => reportValue.City).ToList())
56 }).ToList();
57
58 return reportItems;
59 }
60 }
61 }
File CompanyVsNumberOfPersonReportDefinition.cs:
62 using System.Collections.Generic;
63 using System.Linq;
64
65 namespace SimpleReport.Version2 {
66 internal class CompanyVsNumberOfPersonReportDefinition : BaseReportDefinition, IReportDefinition {
67 public string ReportName { get { return "Company vs number of person"; } }
68
69 public IList<ReportItem> GetReportItems(IList<Row> dataForReport, Month month) {
70 var reportItems = base.GetReportBaseConditions(dataForReport, month)
71 .Select(reportItem => new ReportItem {
72 ValueName = reportItem.Key,
73 Value = reportItem.Sum(reportValue => reportValue.NumberOfPerson).ToString()
74 }).ToList();
75
76 return reportItems;
77 }
78 }
79 }
File CompanyVsPriceReportDefinition.cs:
80 using System.Collections.Generic;
81 using System.Linq;
82
83 namespace SimpleReport.Version2 {
84 internal class CompanyVsPriceReportDefinition : BaseReportDefinition, IReportDefinition {
85 public string ReportName { get { return "Company vs price"; } }
86
87 public IList<ReportItem> GetReportItems(IList<Row> dataForReport, Month month) {
88 var reportItems = base.GetReportBaseConditions(dataForReport, month)
89 .Select(reportItem => new ReportItem {
90 ValueName = reportItem.Key,
91 Value = base.GetPriceAsString(reportItem.Sum(reportValue => reportValue.Price))
92 }).ToList();
93
94 return reportItems;
95 }
96 }
97 }
File IReportDefinition.cs:
98 using System;
99 using System.Collections.Generic;
100
101 namespace SimpleReport.Version2 {
102 internal interface IReportDefinition {
103 string ReportName { get; }
104 Func<Row, string> Aggregate { get; }
105 IList<ReportItem> GetReportItems(IList<Row> dataForReport, Month month);
106 }
107 }
File Month.cs:
108 namespace SimpleReport.Version2 {
109 public enum Month {
110 January = 1,
111 February = 2,
112 March = 3,
113 April = 4,
114 May = 5,
115 June = 6,
116 July = 7,
117 August = 8,
118 September = 9,
119 October = 10,
120 November = 11,
121 December = 12
122 }
123 }
File Report.cs:
124 using System.Collections.Generic;
125
126 namespace SimpleReport.Version2 {
127 public class Report {
128 public string ReportName { get; set; }
129 public IList<ReportItem> Items { get; set; }
130 }
131 }
File ReportFactory.cs:
132 using System;
133 using System.Collections.Generic;
134
135 namespace SimpleReport.Version2 {
136 internal class ReportFactory {
137 private readonly IList<Row> _dataForReport;
138 private readonly ReportType _reportType;
139 private readonly Month _month;
140
141 public ReportFactory(IList<Row> dataForReport, ReportType reportType, Month month) {
142 _dataForReport = dataForReport ?? new List<Row>();
143 _reportType = reportType;
144 _month = month;
145 Validate();
146 }
147
148 public Report GetReport() {
149 var report = new Report();
150 IReportDefinition reportDefinition = GetReportDefinition();
151 report.ReportName = reportDefinition.ReportName;
152 report.Items = reportDefinition.GetReportItems(_dataForReport, _month);
153
154 return report;
155 }
156
157 private IReportDefinition GetReportDefinition() {
158 switch (_reportType) {
159 case ReportType.CompanyVsPrice:
160 return new CompanyVsPriceReportDefinition();
161 case ReportType.CompanyVsCity:
162 return new CompanyVsCityReportDefinition();
163 case ReportType.CityVsNumberOfDays:
164 return new CityVsNumberOfDaysReportDefinition();
165 case ReportType.CompanyVsNumberOfPerson:
166 return new CompanyVsNumberOfPersonReportDefinition();
167 default:
168 throw new NotImplementedException(string.Format("Report for type '{0}' is not implemented!", _reportType));
169 }
170 }
171
172 private void Validate() {
173 if (!Enum.IsDefined(typeof(Month), _month)) {
174 throw new ArgumentOutOfRangeException("month");
175 }
176 }
177 }
178 }
File ReportItem.cs:
179 namespace SimpleReport.Version2 {
180 public class ReportItem {
181 public string ValueName { get; set; }
182 public string Value { get; set; }
183 }
184 }
File Reports.cs:
185 using System.Collections.Generic;
186
187 namespace SimpleReport.Version2 {
188 public class Reports {
189 public Report GenerateReport(ReportType reportType, Month month) {
190 var dataForReport = GetDataForReport();
191 var reportFactory = new ReportFactory(dataForReport, reportType, month);
192 var report = reportFactory.GetReport();
193
194 return report;
195 }
196
197
198 private IList<Row> GetDataForReport() {
199
200 return new List<Row>
201 {
202 new Row { City = "City 1", CompanyName = "Company 1", Month = 1, NumberOfDays = 5, NumberOfPerson = 30, Price = 23045 },
203 new Row { City = "City 1", CompanyName = "Company 1", Month = 2, NumberOfDays = 4, NumberOfPerson = 50, Price = 10000 },
204 new Row { City = "City 1", CompanyName = "Company 1", Month = 3, NumberOfDays = 8, NumberOfPerson = 70, Price = 55000 },
205 new Row { City = "City 2", CompanyName = "Company 1", Month = 4, NumberOfDays = 3, NumberOfPerson = 3, Price = 12000 },
206 new Row { City = "City 2", CompanyName = "Company 2", Month = 1, NumberOfDays = 2, NumberOfPerson = 12, Price = 15000 },
207 new Row { City = "City 2", CompanyName = "Company 2", Month = 6, NumberOfDays = 2, NumberOfPerson = 11, Price = 16000 },
208 new Row { City = "City 3", CompanyName = "Company 1", Month = 7, NumberOfDays = 8, NumberOfPerson = 20, Price = 33000 },
209 new Row { City = "City 3", CompanyName = "Company 3", Month = 8, NumberOfDays = 5, NumberOfPerson = 10, Price = 9000 },
210 new Row { City = "City 1", CompanyName = "Company 1", Month = 9, NumberOfDays = 3, NumberOfPerson = 20, Price = 22000 },
211 new Row { City = "City 1", CompanyName = "Company 1", Month = 10, NumberOfDays = 8, NumberOfPerson = 30, Price = 55000 },
212 };
213 }
214 }
215 }
File ReportType.cs:
216 namespace SimpleReport.Version2 {
217 public enum ReportType {
218 CompanyVsPrice = 1,
219 CompanyVsCity = 2,
220 CityVsNumberOfDays = 3,
221 CompanyVsNumberOfPerson = 4
222 }
223 }
File Row.cs:
224 namespace SimpleReport.Version2 {
225 public class Row {
226 public int Month { get; set; }
227 public string CompanyName { get; set; }
228 public string City { get; set; }
229 public int NumberOfPerson { get; set; }
230 public int NumberOfDays { get; set; }
231 public decimal Price { get; set; }
232 }
233 }
Usage (all versions)
File Program.cs:
1 using SimpleReport;
2 using v1 = SimpleReport.Version1;
3 using v2 = SimpleReport.Version2;
4 using v3 = SimpleReport.Version3;
5 using System;
6
7 namespace SimpleReport {
8 class Program {
9 static void Main(string[] args) {
10
11 Console.WriteLine("version 1:");
12 var myClass = new v1.MyClass();
13 var result = myClass.Process(1, 1, "name 1");
14 foreach (var item in result.Items) {
15 Console.WriteLine(string.Format("{0}:\t{1}", item.ValueName, item.Value));
16 }
17
18 Console.WriteLine("\nversion 2:");
19 var reportFactory = new v2.Reports();
20 var v2report = reportFactory.GenerateReport(v2.ReportType.CompanyVsPrice, v2.Month.January);
21 foreach (var item in v2report.Items) {
22 Console.WriteLine(string.Format("{0}:\t{1}", item.ValueName, item.Value));
23 }
24
25 Console.WriteLine("\nversion 3:");
26 var v3Report = v3.Repository.GetCompanyInfoReport(v3.ReportType.CompanyVsPrice, v3.Months.January);
27 foreach (var entry in v3Report.Entries) {
28 Console.WriteLine(string.Format("{0}:\t{1}", entry.Caption, entry.Value));
29 }
30
31 Console.ReadKey();
32 }
33 }
34 }
Discussion
You see: We compare 13 Files of overall 233 lines of code (original-Version) with 4 Files of 119 lines of code, including several File-Header-Comments, which explain the concerns of the File (my-Version).
In my Version it's obviously, where the data come from: of course from the Repository.
And where my Datamodel is is obviously as well: in the Datamodel-File.
And my Datamodel-Class is named CompanyInfo - because that is what it is (it is not a "Row")
In the original i stepped through all Files searching the data - finally found it in a Class named Reports. (why named "Reports"??)
And Original-Querying the data is spread over 7 other Files, with following hard-to-read-Names:
BaseReportDefinition, CityVsNumberOfDaysReportDefinition, CompanyVsCityReportDefinition, CompanyVsNumberOfPersonReportDefinition, CompanyVsPriceReportDefinition, IReportDefinition, ReportFactory
Note: These 8 Files - in my version they are only one - my Repository. And a specific query is one single line.
But I know, many of you will now downrate me, and disagree, when I postulate:
"The shorter version, with less Files, with rare but meaningful commentation, with better naming, without Interface, baseclass, derived classes, factory and other hoo-haa-stuff - it is easier to understand."
(Ok - I do not know that, but it's my pessimistic expectation.)
This is strange, because I totally agree with the originals authors opinion:
Quote:
Writing clean code is very important. In the future can save a lot of time and nerves. The language you write your code with should look like it was made for the problem. Code should be minimal. There should not be redundant code that does nothing. Variable, methods names should unambiguously indicate their destination. Each constant should have a name that uniquely identifies its destiny. Reading your code should be easy and pleasant.
Let me pressume to give some simple advices for better approaching to these "golden goals":
- Try recognize appropriate wellknown patterns, like "Datamodel", "Repository", "Utility", and use these terms to give your solution structure.
- Keep it simple as long as possible: Eg. just naming a File "Repository" will give good orientation to readers (as long as the File actually contains Repository-Concerns).
- When things become more complex, you can adapt the structure: eg. introduce meaningful named folders, namespaces, or complete projects to manage those specific concerns.
As said: when it becomes complex - not earlier! - Keep Files in human-friendly dimensions:
Thoughtful split Files of more than 300 lines into several smaller ones, each with its own identificable specific concern.
On the other hand - when classes are smaller than 30 lines - feel free to store several of them into one File - as long as the classes are of fairly similar topic.
Because hundreds of Files with nearby no code inside is human-unfriendly as well as one Monster-File. - Don't apply all language-features you've ever heard from to your code.
If you can get by without Interfaces, baseclasses, factories and stuff - please do get by without. - But apply useful language-features, consequently
Especial to avoid repeating code base-classes can be very helpful - that can be seen as the main-concern of baseclasses.
On the other hand - interfaces are code which does nothing - by design. So before introducing one, stop and think about it, whether you really need it.
If there is only one class, implementing the interface, it is mostly dispensable - you can take the class itself instead.
Same if there is a baseclass present, which can do the job.
And same as above: When things become complex, Interfaces may become helpful. Then (not earlier) don't hesitate to introduce them.
Submit an article or tip
News 
Suggestion 
Bug 
Joke 
Praise 
Rant 
Admin