Let's refactor it-OpenSinaAPI
这两天一直在看Clean Code以及 重构,一直想找一些例子练练手。恰巧前两天在玩新浪微博应用的时候看到了陈着写的这个SDK。down下来代码后发现这里面还有很多可以重构的地方,于是花了一个周末实践了一下。
在简单了解了下OAuth的原理及使用后,先确定了重构的范围:
1、基础类oAuthBase.cs不动,因为这个类是由官方提供的。
2、有必要时外部调用接口也需要更改(事实上写到最后几乎对所有的接口都改了)。
开始下手,先看oAuthWebRequest这个函数
public string oAuthWebRequest(Method method, string url, string postData) { string outUrl = ""; string querystring = ""; string ret = ""; postData += "&source=" + appKey; if (method == Method.POST) { if (postData.Length > 0) { NameValueCollection qs = HttpUtility.ParseQueryString(postData); postData = ""; foreach (string key in qs.AllKeys) { if (postData.Length > 0) { postData += "&"; } qs[key] = HttpUtility.UrlEncode(qs[key]); qs[key] = this.UrlEncode(qs[key]); postData += (key + "=" + qs[key]); } if (url.IndexOf("?") > 0) { url += "&"; } else { url += "?"; } url += postData; } } Uri uri = new Uri(url); string nonce = this.GenerateNonce(); string timeStamp = this.GenerateTimeStamp(); //Generate Signature string sig = this.GenerateSignature(uri, this.appKey, this.appSecret, this.token, this.tokenSecret, method.ToString(), timeStamp, nonce, out outUrl, out querystring); querystring += "&oauth_signature=" + HttpUtility.UrlEncode(sig); if (method == Method.POST) { postData = querystring; querystring = ""; } if (querystring.Length > 0) { outUrl += "?"; } if (method == Method.POST || method == Method.GET) ret = _WebRequest(method, outUrl + querystring, postData); return ret; }
这个函数的目的是来创建请求,并且将请求结果返回。
让我们来识别一下这个函数都有哪些bad smell:
1、long method.这个方法足足有70行。
2、Temporary Field.光函数开头声明的临时变量就有3个,函数内部声明的临时变量更是不尽其数。
3、Single Responsibility Principle.函数中充斥着大量的if-else,以及多处判断POST与Get。
4、Higher Cyclomatic Complexity.函数中存在可以避免的嵌套 if-else
重构的方法:
首先根据不同的请求类型(POST/GET)来声明不同的处理方法,为了使客户类调用行为一致,我这里抽取出了一个IHttpRequestMethod接口
public interface IHttpRequestMethod { string Request(string uri, string postData); }
其中Request方法相当于原库中oAuthWebRequest,分别用两个类HttpGet与HttpPost来实现。
然后是将这个方法拆解成为几个小方法。最后的代码如下:
public class HttpPost : BaseHttpRequest { ..... ...... public override string Request(string uri, string postData) { var appendUrl = AppendPostDataToUrl(postData, uri); string outUrl; var querystring= AppendSignatureString(POST, appendUrl, out outUrl); return WebRequest(POST, outUrl, querystring); } ...... }
public class HttpGet : BaseHttpRequest { ....... ...... public override string Request(string uri, string postData) { string outUrl; var queryString = AppendSignatureString(GET, uri, out outUrl); if (queryString.Length > 0) { outUrl += "?"; } return WebRequest(GET, outUrl + queryString); } ...... }
全部代码参见:
http://code.google.com/p/opensinaapi/downloads/detail?name=OpenSinaApi04-18-2011.rar
欢迎各位看官一起动手来进一步重构:)
2011年4月18日 10:32
一点点小建议:
1. 当我看到如下函数调用:
AppendSignatureString(POST, appendUrl, out outUrl); AppendSignatureString(GET, uri, out outUrl);
我会猜想它会根据是哪种协议做判断,然后append;
这暴露出一个坏味道,就是这个函数的职责不单一,根据《clean code》里面的提法,第一个参数类似一个flag argument。
2. 我不推荐使用output argument:
借用Robert C. Martin的话:In general output arguments should be avoided. If your function must change the state
of something, have it change the state of its owning object.
2011年4月18日 10:35
赞同zhangyue的观点
2011年4月18日 10:36
另外,我觉得更有价值的部分是重构的过程,而不仅仅是结果
2011年4月18日 23:37
@xiaodao:
@zhangyue:
你俩肯定没仔细看我的说明,out 参数是因为oAuthBase里的函数参数调用规范,我前提已经说了oAuthBase中的逻辑不动,我也只是follow而已。
2024年1月14日 20:51
Very good topic, similar texts are I do not know if they are as good as your work out. These you will then see the most important thing, the application provides you a website a powerful important internet page