什么是好的代码

我希望能够编写优美的代码。

优美的代码就像一篇散文，易懂易读，而且看起来很漂亮。在《代码之美》一书中，收录了Ruby之父松本行宏的一篇文章，名为《把代码当作文章》，大约表达了同样的含义。Thoughtworks的一位工程师在《软件开发沉思录》一书中提出，每个类的方法最好不要超过5行。最初让我感觉很惊诧，继而觉得不可能。虽然这位工程师言之凿凿，提到在自己参与的项目中，所有代码都完全遵循了这一规范，我仍然表示怀疑。最近，阅读了Robert C. Martin的著作《代码整洁之道》（英文版名为Clean Code），看到Uncle Bob演示的代码，真是漂亮极了。仔细一看，这些好的代码在每个方法中大多数都没有超过5行。诀窍在哪里？那就是重构手法中最常用的Extract Method。进一步讲，如果我们能够为每个类与方法以及变量定义出好的名字，代码确实可以变成一篇散文。当然，是英文散文。

今天，我在重温.NET的序列化时，在MSDN上找到一篇演示Xml序列化的示范代码。或许是因为示范代码的缘故，这一段代码写得极其地不优雅，甚至显得有些丑陋：
 

1. public class Test { 
2.     public static void Main() { 
3.         // Read and write purchase orders. 
4.         Test t = new Test(); 
5.         t.CreatePO("po.xml"); 
6.         t.ReadPO("po.xml"); 
7.     } 
8.  
9.     private void CreatePO(string filename) { 
10.         // Create an instance of the XmlSerializer class; 
11.         // specify the type of object to serialize. 
12.         XmlSerializer serializer = 
13.         new XmlSerializer(typeof(PurchaseOrder)); 
14.         TextWriter writer = new StreamWriter(filename); 
15.         PurchaseOrder po = new PurchaseOrder(); 
16.  
17.         // Create an address to ship and bill to. 
18.         Address billAddress = new Address(); 
19.         billAddress.Name = "Teresa Atkinson"; 
20.         billAddress.Line1 = "1 Main St."; 
21.         billAddress.City = "AnyTown"; 
22.         billAddress.State = "WA"; 
23.         billAddress.Zip = "00000"; 
24.         // Set ShipTo and BillTo to the same addressee. 
25.         po.ShipTo = billAddress; 
26.         po.OrderDate = System.DateTime.Now.ToLongDateString(); 
27.  
28.         // Create an OrderedItem object. 
29.         OrderedItem i1 = new OrderedItem(); 
30.         i1.ItemName = "Widget S"; 
31.         i1.Description = "Small widget"; 
32.         i1.UnitPrice = (decimal)5.23; 
33.         i1.Quantity = 3; 
34.         i1.Calculate(); 
35.  
36.         // Insert the item into the array. 
37.         OrderedItem items = { i1 }; 
38.         po.OrderedItems = items; 
39.         // Calculate the total cost. 
40.         decimal subTotal = new decimal(); 
41.         foreach (OrderedItem oi in items) { 
42.             subTotal += oi.LineTotal; 
43.         } 
44.         po.SubTotal = subTotal; 
45.         po.ShipCost = (decimal)12.51; 
46.         po.TotalCost = po.SubTotal + po.ShipCost; 
47.         // Serialize the purchase order, and close the TextWriter. 
48.         serializer.Serialize(writer, po); 
49.         writer.Close(); 
50.     } 
51.  
52.     protected void ReadPO(string filename) { 
53.         // Create an instance of the XmlSerializer class; 
54.         // specify the type of object to be deserialized. 
55.         XmlSerializer serializer = new XmlSerializer(typeof(PurchaseOrder)); 
56.         /* If the XML document has been altered with unknown  
57.         nodes or attributes, handle them with the  
58.         UnknownNode and UnknownAttribute events.*/ 
59.         serializer.UnknownNode += new 
60.         XmlNodeEventHandler(serializer_UnknownNode); 
61.         serializer.UnknownAttribute += new 
62.         XmlAttributeEventHandler(serializer_UnknownAttribute); 
63.  
64.         // A FileStream is needed to read the XML document. 
65.         FileStream fs = new FileStream(filename, FileMode.Open); 
66.         // Declare an object variable of the type to be deserialized. 
67.         PurchaseOrder po; 
68.         /* Use the Deserialize method to restore the object's state with 
69.         data from the XML document. */ 
70.         po = (PurchaseOrder)serializer.Deserialize(fs); 
71.         // Read the order date. 
72.         Console.WriteLine("OrderDate: " + po.OrderDate); 
73.  
74.         // Read the shipping address. 
75.         Address shipTo = po.ShipTo; 
76.         ReadAddress(shipTo, "Ship To:"); 
77.         // Read the list of ordered items. 
78.         OrderedItem items = po.OrderedItems; 
79.         Console.WriteLine("Items to be shipped:"); 
80.         foreach (OrderedItem oi in items) { 
81.             Console.WriteLine("\t" + 
82.             oi.ItemName + "\t" + 
83.             oi.Description + "\t" + 
84.             oi.UnitPrice + "\t" + 
85.             oi.Quantity + "\t" + 
86.             oi.LineTotal); 
87.         } 
88.         // Read the subtotal, shipping cost, and total cost. 
89.         Console.WriteLine("\t\t\t\t\t Subtotal\t" + po.SubTotal); 
90.         Console.WriteLine("\t\t\t\t\t Shipping\t" + po.ShipCost); 
91.         Console.WriteLine("\t\t\t\t\t Total\t\t" + po.TotalCost); 
92.     } 
93.  
94.     protected void ReadAddress(Address a, string label) { 
95.         // Read the fields of the Address object. 
96.         Console.WriteLine(label); 
97.         Console.WriteLine("\t" + a.Name); 
98.         Console.WriteLine("\t" + a.Line1); 
99.         Console.WriteLine("\t" + a.City); 
100.         Console.WriteLine("\t" + a.State); 
101.         Console.WriteLine("\t" + a.Zip); 
102.         Console.WriteLine(); 
103.     } 
104.  
105.     private void serializer_UnknownNode 
106.     (object sender, XmlNodeEventArgs e) { 
107.         Console.WriteLine("Unknown Node:" + e.Name + "\t" + e.Text); 
108.     } 
109.  
110.     private void serializer_UnknownAttribute 
111.     (object sender, XmlAttributeEventArgs e) { 
112.         System.Xml.XmlAttribute attr = e.Attr; 
113.         Console.WriteLine("Unknown attribute " + 
114.         attr.Name + "='" + attr.Value + "'"); 
115.     } 
116. } 

看看CreatePO()和ReadPO()，多么地冗长。虽然这个实现极为简单，但对于代码的阅读者而言，想要一下子抓住该方法的中心思想，仍然比较困难。此外，方法中的注释也显得多余，因为，代码本身就可以给予很好的说明。

下面，是我对这段代码的重构，大家可以对比对比，是否更加容易阅读呢？

1. public static class PurchaseOrderHandler { 
2.         public static void CreatePurchaseOrder(string filename) { 
3.             PurchaseOrder po = BuildPurchaseOrder(); 
4.             XmlSerializer serializer = new XmlSerializer(typeof(PurchaseOrder)); 
5.             using (var writer = new StreamWriter(filename)) { 
6.                 serializer.Serialize(writer, po); 
7.             } 
8.         } 
9.  
10.         private static PurchaseOrder BuildPurchaseOrder() { 
11.             Address address = CreateAddress(); 
12.             OrderedItem i1 = CreateOrderedItem(); 
13.             OrderedItem items = { i1 }; 
14.  
15.             PurchaseOrder po = new PurchaseOrder(); 
16.             po.ShipTo = address; 
17.             po.OrderDate = System.DateTime.Now.ToLongDateString(); 
18.             po.OrderedItems = items; 
19.             po.SubTotal = CalculateSubTotal(items); 
20.             po.ShipCost = (decimal)12.51; 
21.             po.TotalCost = po.SubTotal + po.ShipCost; 
22.  
23.             return po; 
24.         } 
25.  
26.         private static decimal CalculateSubTotal(OrderedItem items) { 
27.             decimal subTotal = new decimal(); 
28.             foreach (OrderedItem oi in items) { 
29.                 subTotal += oi.LineTotal; 
30.             } 
31.  
32.             return subTotal; 
33.         } 
34.  
35.  
36.         private static OrderedItem CreateOrderedItem() { 
37.             OrderedItem i1 = new OrderedItem(); 
38.             i1.ItemName = "Widget S"; 
39.             i1.Description = "Small widget"; 
40.             i1.UnitPrice = (decimal)5.23; 
41.             i1.Quantity = 3; 
42.             i1.Calculate(); 
43.             return i1; 
44.         } 
45.  
46.         private static Address CreateAddress() { 
47.             Address billAddress = new Address(); 
48.             billAddress.Name = "Bruce Zhang"; 
49.             billAddress.Line1 = "1 Main St."; 
50.             billAddress.City = "Chong Qing"; 
51.             billAddress.State = "Chong Qing"; 
52.             billAddress.Zip = "400000"; 
53.  
54.             return billAddress; 
55.         } 
56.  
57.         public static void ReadPurchaseOrder(string filename) { 
58.             XmlSerializer serializer = new XmlSerializer(typeof(PurchaseOrder)); 
59.  
60.             serializer.UnknownNode += new XmlNodeEventHandler(serializer_UnknownNode); 
61.             serializer.UnknownAttribute += new XmlAttributeEventHandler(serializer_UnknownAttribute); 
62.  
63.             FileStream fs = new FileStream(filename, FileMode.Open); 
64.  
65.             PurchaseOrder po; 
66.             po = (PurchaseOrder)serializer.Deserialize(fs); 
67.             PurchaseOrderPrinter.PrintPurchaseOrder(po); 
68.         } 
69.  
70.  
71.         private static void serializer_UnknownNode 
72.         (object sender, XmlNodeEventArgs e) { 
73.             Console.WriteLine("Unknown Node:" + e.Name + "\t" + e.Text); 
74.         } 
75.  
76.         private static void serializer_UnknownAttribute 
77.         (object sender, XmlAttributeEventArgs e) { 
78.             System.Xml.XmlAttribute attr = e.Attr; 
79.             Console.WriteLine("Unknown attribute " + 
80.             attr.Name + "='" + attr.Value + "'"); 
81.         } 
82.  
83.         private static class PurchaseOrderPrinter { 
84.             public static void PrintPurchaseOrder(PurchaseOrder po) { 
85.                 PrintOrderDate(po); 
86.                 PrintAddress(po.ShipTo); 
87.                 PrintOrderedItem(po.OrderedItems); 
88.                 PrintOrderCost(po); 
89.             } 
90.  
91.             private static void PrintOrderCost(PurchaseOrder po) { 
92.                 Console.WriteLine("\t\t\t\t\t Subtotal\t" + po.SubTotal); 
93.                 Console.WriteLine("\t\t\t\t\t Shipping\t" + po.ShipCost); 
94.                 Console.WriteLine("\t\t\t\t\t Total\t\t" + po.TotalCost); 
95.             } 
96.  
97.             private static void PrintOrderDate(PurchaseOrder po) { 
98.                 Console.WriteLine("OrderDate: " + po.OrderDate); 
99.             } 
100.  
101.             private static void PrintOrderedItem(OrderedItem items) { 
102.                 Console.WriteLine("Items to be shipped:"); 
103.                 foreach (OrderedItem oi in items) { 
104.                     Console.WriteLine("\t" + 
105.                     oi.ItemName + "\t" + 
106.                     oi.Description + "\t" + 
107.                     oi.UnitPrice + "\t" + 
108.                     oi.Quantity + "\t" + 
109.                     oi.LineTotal); 
110.                 } 
111.             } 
112.  
113.             private static void PrintAddress(Address a) { 
114.                 // Read the fields of the Address object. 
115.                 Console.WriteLine("Ship To:"); 
116.                 Console.WriteLine("\t" + a.Name); 
117.                 Console.WriteLine("\t" + a.Line1); 
118.                 Console.WriteLine("\t" + a.City); 
119.                 Console.WriteLine("\t" + a.State); 
120.                 Console.WriteLine("\t" + a.Zip); 
121.                 Console.WriteLine(); 
122.             } 
123.         } 
124.     } 

阅读代码时，我们可以先关注最主要的方法，即CreatePurchaseOrder()和ReadPurchaseOrder()方法。如果并不希望了解过多构造PO对象的细节，通过阅读这样简短的方法，可以很容易地抓住这两个方法的实现，那就是通过构建一个PO对象，进行序列化，而在反序列化时，将获得的PO对象信息打印出来。

其实，糟糕的代码不一定就是初学者的“专利”，让我们看看NHibernate中的一段代码：

1. public SessionFactoryImpl(Configuration cfg, IMapping mapping, Settings settings, EventListeners listeners) 
2. { 
3.     Init(); 
4.     log.Info("building session factory"); 
5.  
6.     properties = new Dictionary<string, string>(cfg.Properties); 
7.     interceptor = cfg.Interceptor; 
8.     this.settings = settings; 
9.     sqlFunctionRegistry = new SQLFunctionRegistry(settings.Dialect, cfg.SqlFunctions); 
10.     eventListeners = listeners; 
11.     filters = new Dictionary<string, FilterDefinition>(cfg.FilterDefinitions); 
12.     if (log.IsDebugEnabled) 
13.     { 
14.         log.Debug("Session factory constructed with filter configurations : " + CollectionPrinter.ToString(filters)); 
15.     } 
16.  
17.     if (log.IsDebugEnabled) 
18.     { 
19.         log.Debug("instantiating session factory with properties: " + CollectionPrinter.ToString(properties)); 
20.     } 
21.  
22.     try 
23.     { 
24.         if (settings.IsKeywordsImportEnabled) 
25.         { 
26.             SchemaMetadataUpdater.Update(this); 
27.         } 
28.         if (settings.IsAutoQuoteEnabled) 
29.         { 
30.             SchemaMetadataUpdater.QuoteTableAndColumns(cfg); 
31.         } 
32.     } 
33.     catch (NotSupportedException) 
34.     { 
35.         // Ignore if the Dialect does not provide DataBaseSchema  
36.     } 
37.  
38.     #region Caches 
39.     settings.CacheProvider.Start(properties); 
40.     #endregion 
41.  
42.     #region Generators 
43.     identifierGenerators = new Dictionary<string, IIdentifierGenerator>(); 
44.     foreach (PersistentClass model in cfg.ClassMappings) 
45.     { 
46.         if (!model.IsInherited) 
47.         { 
48.             IIdentifierGenerator generator = 
49.                 model.Identifier.CreateIdentifierGenerator(settings.Dialect, settings.DefaultCatalogName, 
50.                                                            settings.DefaultSchemaName, (RootClass) model); 
51.  
52.             identifierGenerators[model.EntityName] = generator; 
53.         } 
54.     } 
55.     #endregion 
56.  
57.     #region Persisters 
58.  
59.     Dictionary<string, ICacheConcurrencyStrategy> caches = new Dictionary<string, ICacheConcurrencyStrategy>(); 
60.     entityPersisters = new Dictionary<string, IEntityPersister>(); 
61.     implementorToEntityName = new Dictionary<System.Type, string>(); 
62.  
63.     Dictionary<string, IClassMetadata> classMeta = new Dictionary<string, IClassMetadata>(); 
64.  
65.     foreach (PersistentClass model in cfg.ClassMappings) 
66.     { 
67.         model.PrepareTemporaryTables(mapping, settings.Dialect); 
68.         string cacheRegion = model.RootClazz.CacheRegionName; 
69.         ICacheConcurrencyStrategy cache; 
70.         if (!caches.TryGetValue(cacheRegion, out cache)) 
71.         { 
72.             cache = 
73.                 CacheFactory.CreateCache(model.CacheConcurrencyStrategy, cacheRegion, model.IsMutable, settings, properties); 
74.             if (cache != null) 
75.             { 
76.                 caches.Add(cacheRegion, cache); 
77.                 allCacheRegions.Add(cache.RegionName, cache.Cache); 
78.             } 
79.         } 
80.         IEntityPersister cp = PersisterFactory.CreateClassPersister(model, cache, this, mapping); 
81.         entityPersisters[model.EntityName] = cp; 
82.         classMeta[model.EntityName] = cp.ClassMetadata; 
83.  
84.         if (model.HasPocoRepresentation) 
85.         { 
86.             implementorToEntityName[model.MappedClass] = model.EntityName; 
87.         } 
88.     } 
89.     classMetadata = new UnmodifiableDictionary<string, IClassMetadata>(classMeta); 
90.  
91.     Dictionary<string, ISet<string>> tmpEntityToCollectionRoleMap = new Dictionary<string, ISet<string>>(); 
92.     collectionPersisters = new Dictionary<string, ICollectionPersister>(); 
93.     foreach (Mapping.Collection model in cfg.CollectionMappings) 
94.     { 
95.         ICacheConcurrencyStrategy cache = 
96.             CacheFactory.CreateCache(model.CacheConcurrencyStrategy, model.CacheRegionName, model.Owner.IsMutable, settings, 
97.                                      properties); 
98.         if (cache != null) 
99.         { 
100.             allCacheRegions[cache.RegionName] = cache.Cache; 
101.         } 
102.         ICollectionPersister persister = PersisterFactory.CreateCollectionPersister(cfg, model, cache, this); 
103.         collectionPersisters[model.Role] = persister; 
104.         IType indexType = persister.IndexType; 
105.         if (indexType != null && indexType.IsAssociationType && !indexType.IsAnyType) 
106.         { 
107.             string entityName = ((IAssociationType) indexType).GetAssociatedEntityName(this); 
108.             ISet<string> roles; 
109.             if (!tmpEntityToCollectionRoleMap.TryGetValue(entityName, out roles)) 
110.             { 
111.                 roles = new HashedSet<string>(); 
112.                 tmpEntityToCollectionRoleMap[entityName] = roles; 
113.             } 
114.             roles.Add(persister.Role); 
115.         } 
116.         IType elementType = persister.ElementType; 
117.         if (elementType.IsAssociationType && !elementType.IsAnyType) 
118.         { 
119.             string entityName = ((IAssociationType) elementType).GetAssociatedEntityName(this); 
120.             ISet<string> roles; 
121.             if (!tmpEntityToCollectionRoleMap.TryGetValue(entityName, out roles)) 
122.             { 
123.                 roles = new HashedSet<string>(); 
124.                 tmpEntityToCollectionRoleMap[entityName] = roles; 
125.             } 
126.             roles.Add(persister.Role); 
127.         } 
128.     } 
129.     Dictionary<string, ICollectionMetadata> tmpcollectionMetadata = new Dictionary<string, ICollectionMetadata>(collectionPersisters.Count); 
130.     foreach (KeyValuePair<string, ICollectionPersister> collectionPersister in collectionPersisters) 
131.     { 
132.         tmpcollectionMetadata.Add(collectionPersister.Key, collectionPersister.Value.CollectionMetadata); 
133.     } 
134.     collectionMetadata = new UnmodifiableDictionary<string, ICollectionMetadata>(tmpcollectionMetadata); 
135.     collectionRolesByEntityParticipant = new UnmodifiableDictionary<string, ISet<string>>(tmpEntityToCollectionRoleMap); 
136.     #endregion 
137.  
138.     #region Named Queries 
139.     namedQueries = new Dictionary<string, NamedQueryDefinition>(cfg.NamedQueries); 
140.     namedSqlQueries = new Dictionary<string, NamedSQLQueryDefinition>(cfg.NamedSQLQueries); 
141.     sqlResultSetMappings = new Dictionary<string, ResultSetMappingDefinition>(cfg.SqlResultSetMappings); 
142.     #endregion 
143.  
144.     imports = new Dictionary<string, string>(cfg.Imports); 
145.  
146.     #region after *all* persisters and named queries are registered 
147.     foreach (IEntityPersister persister in entityPersisters.Values) 
148.     { 
149.         persister.PostInstantiate(); 
150.     } 
151.     foreach (ICollectionPersister persister in collectionPersisters.Values) 
152.     { 
153.         persister.PostInstantiate(); 
154.     } 
155.     #endregion 
156.  
157.     #region Serialization info 
158.  
159.     name = settings.SessionFactoryName; 
160.     try 
161.     { 
162.         uuid = (string) UuidGenerator.Generate(null, null); 
163.     } 
164.     catch (Exception) 
165.     { 
166.         throw new AssertionFailure("Could not generate UUID"); 
167.     } 
168.  
169.     SessionFactoryObjectFactory.AddInstance(uuid, name, this, properties); 
170.  
171.     #endregion 
172.  
173.     log.Debug("Instantiated session factory"); 
174.  
175.     #region Schema management 
176.     if (settings.IsAutoCreateSchema) 
177.     { 
178.         new SchemaExport(cfg).Create(false, true); 
179.     } 
180.  
181.     if ( settings.IsAutoUpdateSchema ) 
182.     { 
183.         new SchemaUpdate(cfg).Execute(false, true); 
184.     } 
185.     if (settings.IsAutoValidateSchema) 
186.     { 
187.          new SchemaValidator(cfg, settings).Validate(); 
188.     } 
189.     if (settings.IsAutoDropSchema) 
190.     { 
191.         schemaExport = new SchemaExport(cfg); 
192.     } 
193.     #endregion 
194.  
195.     #region Obtaining TransactionManager 
196.     // not ported yet 
197.     #endregion 
198.  
199.     currentSessionContext = BuildCurrentSessionContext(); 
200.  
201.     if (settings.IsQueryCacheEnabled) 
202.     { 
203.         updateTimestampsCache = new UpdateTimestampsCache(settings, properties); 
204.         queryCache = settings.QueryCacheFactory.GetQueryCache(null, updateTimestampsCache, settings, properties); 
205.         queryCaches = new ThreadSafeDictionary<string, IQueryCache>(new Dictionary<string, IQueryCache>()); 
206.     } 
207.     else 
208.     { 
209.         updateTimestampsCache = null; 
210.         queryCache = null; 
211.         queryCaches = null; 
212.     } 
213.  
214.     #region Checking for named queries 
215.     if (settings.IsNamedQueryStartupCheckingEnabled) 
216.     { 
217.         IDictionary<string, HibernateException> errors = CheckNamedQueries(); 
218.         if (errors.Count > 0) 
219.         { 
220.             StringBuilder failingQueries = new StringBuilder("Errors in named queries: "); 
221.             foreach (KeyValuePair<string, HibernateException> pair in errors) 
222.             { 
223.                 failingQueries.Append('{').Append(pair.Key).Append('}'); 
224.                 log.Error("Error in named query: " + pair.Key, pair.Value); 
225.             } 
226.             throw new HibernateException(failingQueries.ToString()); 
227.         } 
228.     } 
229.     #endregion 
230.  
231.     Statistics.IsStatisticsEnabled = settings.IsStatisticsEnabled; 
232.  
233.     // EntityNotFoundDelegate 
234.     IEntityNotFoundDelegate enfd = cfg.EntityNotFoundDelegate; 
235.     if (enfd == null) 
236.     { 
237.         enfd = new DefaultEntityNotFoundDelegate(); 
238.     } 
239.     entityNotFoundDelegate = enfd; 
240. } 

这是类SessionFactoryImpl（它实现了ISessionFactoryImplementor接口）的构造函数，其目的时是通过Configuration以及Setting中的某些值，去初始化SessionFactoryImpl，然后构建该类的对象。坦白说，我从来没有看过如此“浩瀚无垠”的构造函数。幸好，Visual Studio提高了Region，否则，更让人头疼。（我在想，既然代码的编写者已经利用了Region来分割实现，为何不进一步将其分割为小的方法呢？）

看这样的代码，我们能够轻易读懂吗？

拙劣代码可谓遗患无穷。在《程序员修炼之道》一书中，提到了所谓“破窗效应”，即“没修复的破窗，导致更多的窗户被打破”。丑陋的代码如果只有一个小的片段，看似无关紧要，就像一幢大楼的一扇破窗一般容易让人忘记。随着时间的推移，当这些丑陋代码不知不觉蔓延到整个项目中时，我们才发现这一幢大楼已经满目疮痍了。“一屋不扫，何以扫天下”，程序员应该从小处着手，未来才可能写出优雅的代码。