主要目的教大家重構的技巧
1.重複的程式碼duplicated code
2.If的巢狀結構
寫這篇文章的動機，主要是無聊逛specflow的一篇問題，覺得很有趣所以想來寫一下
http://stackoverflow.com/questions/8417145/best-practice-for-refactoring-methods
簡述一下問題，作者想要重構以下這段程式碼。
private static bool Foo()
{
bool result = false;
if (DoMehod1())
{
if (DoMehod2())
{
if (DoMethod3())
{
result = true;
}
else
{
Console.WriteLine("DoMethod3 Failed");
}
}
else
{
Console.WriteLine("DoMethod2 Failed");
}
}
else
{
Console.WriteLine("DoMethod1 Failed");
}
return result;
}
因為原作者字其實有打錯，先做修正
我先把這個Function移到MethodClass 。
為了測試方便，同時不影響我們要重構的目的。我作了以下三點修正
1.我把宣告 Foo()為public
2.同時因為Foo() 有Console.WriteLine()這個靜態方法 也為了測試方便，支援讓Foo()回傳錯誤訊息，如果成功則回傳"Sucess!"
Foo()最後回傳一個FooData的一個Data Object
3.實作DoMethod1，DoMethod2，DoMethod3並宣告為protected virtual method(為了之後編寫單元測試)
修正後程式碼如下
class FooData
{
public bool Result;
public string Message;
}
class MethodClass
{
public FooData Foo()
{
bool result = false;
string message = "Sucess";
if (DoMethod1())
{
if (DoMethod2())
{
if (DoMethod3())
{
result = true;
}
else
{
message = "DoMethod3 Failed";
}
}
else
{
message = "DoMethod2 Failed";
}
}
else
{
message = "DoMethod1 Failed";
}
return new FooData() { Result = result, Message = message };
}
protected virtual bool DoMethod1()
{
return true;
}
protected virtual bool DoMethod2()
{
return true;
}
protected virtual bool DoMethod3()
{
return true;
}
}
接著我們來建立測試程式碼!測試為重構之本!
1.我建立一個MockObject Class:MockMethodClass，同時建立它的Constructor 來初始化 Method1~3的Result 1~3是True or False
public MockMethodClass(bool result1, bool result2, bool result3)
{
Method1Result = result1;
Method2Result = result2;
Method3Result = result3;
}
2.為了好一點的可讀性，我把 TestCase分成四種
1.Method1 Fail
2.Method2 Fail
3.Method3 Fail
4.All Method Sucess
以下是完整的測試程式碼，使用Nunit 2.6.4版本
using NUnit.Framework;
using Refactor_Section1;
namespace Refactor_Section1UnitTest
{
[TestFixture]
public class MethodClassTest
{
[TestCase(false, true, true, "DoMethod1 Failed")]
[TestCase(false, true, false, "DoMethod1 Failed")]
[TestCase(false, false, true, "DoMethod1 Failed")]
[TestCase(false, false, false, "DoMethod1 Failed")]
public void Foo_Method1Fail_ShouldMatchMessage(bool result1, bool result2, bool result3, string expected)
{
MockMethodClass mockMethodClass = new MockMethodClass(result1, result2, result3);
var fooData = mockMethodClass.Foo();
Assert.AreEqual(false, fooData.Result);
Assert.AreEqual(expected, fooData.Message);
}
[TestCase(true, false, true, "DoMethod2 Failed")]
[TestCase(true, false, false, "DoMethod2 Failed")]
public void Foo_Method2Fail_ShouldMatchMessage(bool result1, bool result2, bool result3, string expected)
{
MockMethodClass mockMethodClass = new MockMethodClass(result1, result2, result3);
var fooData = mockMethodClass.Foo();
Assert.AreEqual(false, fooData.Result);
Assert.AreEqual(expected, fooData.Message);
}
[TestCase(true, true, false, "DoMethod3 Failed")]
public void Foo_Method3Fail_ShouldMatchMessage(bool result1, bool result2, bool result3, string expected)
{
MockMethodClass mockMethodClass = new MockMethodClass(result1, result2, result3);
var fooData = mockMethodClass.Foo();
Assert.AreEqual(false, fooData.Result);
Assert.AreEqual(expected, fooData.Message);
}
[TestCase(true, true, true, "Sucess")]
public void Foo_AllMethodSucess_ShouldMatchMessage(bool result1, bool result2, bool result3, string expected)
{
MockMethodClass mockMethodClass = new MockMethodClass(result1, result2, result3);
var fooData = mockMethodClass.Foo();
Assert.AreEqual(true, fooData.Result);
Assert.AreEqual(expected, fooData.Message);
}
class MockMethodClass : MethodClass
{
public bool Method1Result = true;
public bool Method2Result = true;
public bool Method3Result = true;
public MockMethodClass(bool result1, bool result2, bool result3)
{
Method1Result = result1;
Method2Result = result2;
Method3Result = result3;
}
protected override bool DoMethod1()
{
return Method1Result;
}
protected override bool DoMethod2()
{
return Method2Result;
}
protected override bool DoMethod3()
{
return Method3Result;
}
}
}
}
當然全部通過測試XD
搞定測試後~我們終於可以開始Refactor了!
首先，這個Foo()方法 主要的邏輯 是由Method1->Method2->Mthod3一層一層下去判斷，只要其中一個Method失敗，便assign Message 並且結果為flase ，之後就不用再走下一個方法判斷。
這個if else的巢狀結構要是日後有新Feature再發展下去實在有點驚人~嚇屬寶寶了。
所以為了對付這段程式碼，我們可以使用反向條件敘述，讓結構變單純。作法如下
public FooData Foo()
{
if (!DoMethod1())
{
return new FooData() { Result = false, Message = "DoMethod1 Failed" };
}
if (!DoMethod2())
{
return new FooData() { Result = false, Message = "DoMethod2 Failed" };
}
if (!DoMethod3())
{
return new FooData() { Result = false, Message = "DoMethod3 Failed" };
}
return new FooData() { Result = true, Message = "Sucess" };
}
喔耶~爽拉~經過這種反向敘述，巢狀結構燒毀囉~!變成3個if+return 的物件的形式。
同時我有再inline variable讓程式碼變得更簡潔。(當然~測試都要通過才算重構無誤喔~哥當然是成功的XDDDD)
基本上重構到這一步已經算是OK了，但身為Coder，為了Coder的魂，沒有最好只有更好~還有一大塊duplicated code可以改善!
首先，我們可以發現，每個if內部都長得很像，只是Message不太一樣而已，我們可以嘗試用Extract Method去擷取相同的程式碼架構試試看會如何
來囉，經過這個想法，程式碼如下:
public FooData Foo()
{
if (!DoMethod1())
{
return HandleUnsucessFoo("DoMethod1 Failed");
}
if (!DoMethod2())
{
return HandleUnsucessFoo("DoMethod2 Failed" );
}
if (!DoMethod3())
{
return HandleUnsucessFoo("DoMethod3 Failed");
}
return new FooData() { Result = true, Message = "Sucess" };
}
private FooData HandleUnsucessFoo(string msg)
{
return new FooData() { Result = false, Message = msg };
}
提取一個Method 叫做HandleUnsucessFoo() 專門回傳失敗FooData物件
最後仍然還是有重複的程式碼，我們要用迴圈結構來消除，作法如下
public FooData Foo()
{
foreach (var methodInfo in GetMethodInfos())
{
if (!methodInfo.Value)
{
return HandleUnsucessFoo(methodInfo.Key);
}
}
return new FooData() { Result = true, Message = "Sucess" };
}
private Dictionary<string, bool> GetMethodInfos()
{
return new Dictionary<string, bool>()
{
{"DoMethod1 Failed", DoMethod1()},
{"DoMethod2 Failed", DoMethod2()},
{"DoMethod3 Failed", DoMethod3()},
};
}
1.我把Foo要做的Methods跟該Method的Message先弄成一個Dictionary的資料結構
(我承認偷懶了 key跟value反著用，為了不讓key重複產生，而且程式碼比較難理解，比較好的做法是產生一個MethodInfo的物件來接值但.....我懶得改了)，
好讓之後可以付用同樣的程式碼結構
2.同時把這段擷取成一個Method:GetMethodInfos()
最後跑一下測試，PASS~於是重構終於完成!!跟原本的程式碼是不是十萬八千里呢!其實也沒想像中複雜。
重構後程式碼變得更加好理解，而且有單元測試保護，不怕改錯程式碼邏輯，假使以後Foo要加其他Method4,5,6,.....也都只要在MethodInfo加上新的MethodInfo，就可以拉
爽拉~終於打完這篇了~時間一下子就噴了QQ
希望大家有收穫囉~我們下回見囉~(OS:上一篇是一年前...)~