[原文链接]
导读:本文中笔者为我们介绍了如何实施重构以及在一次重构之后应该牢记的最佳实践,还包括笔者最近重构或review代码时发现的一些问题。
提到让你重构一个功能模块,提到让你去修改一个别人的bug,阅读别人的代码,你第一反应是什么?惧怕?想骂人?面对一个代码垃圾场,我想大家都会有些不安,尤其是面对测试团队,上线的压力。
重构需要勇气
面对一个代码垃圾场,光抱怨是没用的。需要鼓起勇气去面对重构。
如何实施重构
首先,需要大家思考以下一些问题:
(1)代码垃圾场是如何形成的?
团队中有很多菜鸟?团队执行力不强?团队管理跟不上?企业文化问题?其实一个代码垃圾场的形成和这些都有关。试问一个新人在没有培训和老员工带领的情况下如何写出基于本企业框架等的规范代码?一个团队没有形成结对,review代码,控制质量的意识如何走向卓越?一个企业不重视技术人员福利待遇,如何能招募和留住优秀的员工?一个终日想着加薪,而自己技术水平和做事能力却半斤八量,却始终没有想着做好本职工作的员工就是代码垃圾场的主力制造者。
(2)重构实践方法
(a)把握全局,先了解业务
(b)深入理解团队基于的开发平台
(c)阅读代码,清除冗余代码,重构变量名等,删除不一致或无效的注释,补写注释,为下一步做准备
(d)将通用的代码块提升为方法
(e)小步前进,随时测试
在一次重构之后应该牢记的最佳实践:
(1)每个程序员都应该不断加强自己的编程思想,注重编程质量
(2)重构要随时进行
(3)避免过度设计。模式是把双刃剑。举例来说,比如权限验证,在开发环境我们一般用Mock实现,在实际测试环境我们用真实实现,这里我感觉用一个简单的if-else胜过使用Providers模式,因为这个场景太简单了,而且可预期,要么真实实现,要么Mock实现。
下面记录下我最近重构或review代码时发现的一些问题:
1.对ADO.net基础理解不透,或者是对业务理解不透
比如ExecuteScalar,我们在查询数据库中的单个值时就常用这个方法。但是有两点要注意:
(a)如果查询的记录不存在,则返回null
(b)如果查询的记录存在,但在数据库中是null,则返回DBNull.Value
因此我们在使用时要结合业务做容错处理,比如在不存在或存在为DBNull.Value时就可以配置默认值等。如下:
以下是引用片段: //step0:CoreData default value is string.empty string coreData = string.Empty; //step1:read CoreData object result = SQLiteHelper.ExcuteScalar("SELECT CoreData FROM TestTable LIMIT 1"); if (result != null && result != System.DBNull.Value) { coreData = result.ToString(); } //step2:processing CoreData |
2.对值类型和引用类型理解不够,也可以说对代码容错处理过度
下面是一同事的代码断:
以下是引用片段: int selectedIndex = 0 if (selectedIndex != null) { } |
3.下面这个就是容错处理不够,主要是同事不了解发布的过程。发布时可能忘记加相关的配置项的哦。
如果在配置文件中一时忘记配置,就势必要遇到NullReferenceException.
以下是引用片段: App.config <?xml version="1.0" encoding="utf-8" ?> <configuration> <connectionStrings> <add name="SQLite.ConnectionString" connectionString="Data Source={0}ECMarketPlace.db; Version=3;New=False;Connection Timeout=3; Compress=True;Cache Size=1024;" providerName="System.Data.SQLite"/> </connectionStrings> <appSettings> <!--<add key="IsRelease" value="N"/>--> </appSettings> </configuration> C#(容错处理不够) //get release config string isRelease = ConfigurationManager.AppSettings["IsRelease"]; if (isRelease.ToUpper() == "Y") { //processing release logic } C#(适当的容错处理) //get release config string isReleaseConfig = ConfigurationManager.AppSettings["IsRelease"]; string isRelease = isReleaseConfig == null ? "N" : isReleaseConfig.ToUpper(); if (isRelease == "Y") { //processing release logic } |
4.约定优于配置
对于百年就不得变的模版名字就不用写在配置文件中了。
5.在构造SSB消息时,最好不要用Serialization的方式。这是因为重构随时在进行,一个属性名有一天不小时被别人重构了,在序列化出来的XML消息中自然就变了,但是在处理SSB消息的存储过程中XPath路径没变,当然就取不到参数值了。建议用System.Xml.Linq.XElement构造SSB消息。示例如下:
以下是引用片段: const string DECIMAL_FORMAT = "F2"; static XCData XCData(string value) { return new XCData(value != null ? value.Trim() : string.Empty); } static string SSBMessageNode_ProductList(List<Product> products) { XElement result = new XElement("ProductList"); if (products != null) { foreach (var p in products) { result.Add(new XElement("Product", new XElement("ID", p.ID.ToString()), new XElement("Name", XCData(p.Name)), new XElement("Url", XCData(p.Url)), new XElement("SellPrice", p.SellPrice.ToString(DECIMAL_FORMAT)), new XElement("Inventory", p.Inventory.ToString()) )); } } return result.ToString(); } |
使用方法:
以下是引用片段: 代码 List<Product> products = new List<Product>(); products.Add(new Product { ID=1, Name = "格兰仕怡宝系列大1P壁挂式家用冷暖光波空调KFR-26GW/DLP7-130<1>", Url = "http://www.360buy.com/product/213683.html", SellPrice=3699.12M, Inventory=9999 }); string ssbMessageNode_ProductList = SSBMessageNode_ProductList(products); |
输出结果示例:
以下是引用片段: <ProductList> <Product> <ID>1</ID> <Name><![CDATA[格兰仕怡宝系列大1P壁挂式家用冷暖光波空调KFR-26GW/DLP7-130<1>]]></Name> <Url><![CDATA[http://www.360buy.com/product/213683.html]]></Url> <SellPrice>3699.12</SellPrice> <Inventory>9999</Inventory> </Product> </ProductList> |
嗯,先总结到这里。谢谢。:)