重構教程
程式語言：Objective-C
IDE: Xcode 5
有關重構範例：
[來源] IOS7QRCodeDemo
[功能說明] 使用 iOS7 AVFoundation framework 編寫 QRCode 讀取功能。
[特色]不需依賴其他第三方框架
重構理由：
1. 可讀性/可維護性
2. 延展性 (封裝性/模組化能力/置換與轉換能力)
3. 安全性
4. 最佳化能力
重構步驟：
步驟一：將原始碼專案目錄建立 SCM Repository。這裡使用的是 Git。
理由：以利還原及追蹤。這是重要的第一步。
> cd ${QR Codes} 目錄
> git init .
步驟二：審視並調整群組結構
理由：更好的可讀性，加速閱讀及查找速度。
說明：
1. 和主流程相關的部份，我收納在 Main 群組，並以建議閱讀的順序排列。
2. 把 View 和 Model 分開收放。
3. 群組之間的順序也是以建議閱讀的順序排列。
4. 群組的分類方式，可依循的規範參考有以下數種選擇，我們可以擇一或混合使用：
(1) 依照團隊慣例 (coding guideline)
(2) 參考泛用性高的框架 (ex: Ruby on Rails)
(3) 參考建置工具的標準建議 (ex: Maven)
步驟三：審視資訊隱藏
理由：沒必要放在 .h 檔中變成 public 資訊的程式內容，我把它們移至 .m 檔中。
(紅色部分表示「刪除」，綠色部分表示「加入」，下圖表示：將部份程式碼從 ViewController.h 移至 ViewController.m )
步驟四：重構 _previewLayer.frame = _previewView.bounds
理由：這行程式碼的用意是讓 _previewLayer 的尺寸相等於 _previewView，我打算讓它能被更快看懂。例如，我希望重構結果是這樣：
[self makeSameSize:_previewView resizeView:_previewLayer];
說明：
1. 由於原式是無法進行方法抽取的，所以要分幾個程式進行重構。第一招是：加入區域變數！(紅色部份是原式，我改寫成綠色部份。然後編譯、測試。)
2. 然後進行方法的抽出。
3. 然後再把區域變數 inline 回去。
4. 可以看出來：42~43行的兩個區域變數已經不需要了，就拿掉它吧。
步驟五：協調單一函式內敘句的質量密度
理由：
1. 39, 40 行都是訊息傳送/方法呼叫，但是 42 ~ 53 行的程式碼是述句，在物理結構上就不一致。
2. 由上圖上紅色圓角矩形所示，viewDidLoad 內總共做了5件事，其中3件事是以述句的組合構成。若是把5件事都以同樣的密度陳述，用更直覺得方式表達會更好。
說明：
1. 首先，先把第3、第4件事，用抽出方法進行重構，結果如下：
先刻意留下第 61 行的第5件事，只把第 55 行及第 57 行的第3、第4件事抽出來後，停在這兒說明一件事。請看看第45行的註解，該註解在 registerNotificationForCameraOnOff 方法抽出後，顯示不太重要了！於是，我們可以將它進行移除。
這個動作將代出重構後程式碼的幾個象徵：
(1) 每個 method 的行數均不多，而且每一行的執行目的，都有一個單一、難再切割的目的。
(2) 幾乎可以拿掉大部份的註解：因為方法名稱已經足夠傳達程式碼的意圖了。
最後的結果如下：
這裡要注意部分是：為程式碼的閱讀者的習慣提出設想。
一般來說，我們對於文字的閱讀方向是：由上而下，由左至右。為了能讓程式碼的閱讀者有要順暢的閱讀體驗，把 viewDidLoad 方法提至最上方。原因很簡單：閱碼者應該會想知道 viewDidLoad 做了哪幾件事？然後視其需要，再決定要不要 drill down 讀下去。
然後，方法的呼叫順序與方法的擺放順序盡可能一致，有利於查找。
到了這個步驟完成，會發現重構過程式碼應該易讀、看起來乾淨，沒有不需要的註解(通常註解會用不同顏色區分，也會干擾閱讀感覺)。當然，美感方面有個人的差異，這點只能說重構者要 do my best。
步驟六：思考、追究並改善下列程式碼
理由：在 setupCaptureSession 方法中，有下列程式碼，
if (_captureSession)
return;
這是什麼意思呢？更直覺的寫法應該是:
if (_captureSession!=nil)
return;
那為什麼 setupCaptureSession 方法中的 _captureSession 何時會等於 nil 呢？
單看程式碼是找不出原因的。但是，若去思考 setupCaptureSession 的被呼叫位置 : viewDidLoad 方法內，大約就可以找到原因：是在防止出現 memory warning 發生時可能造成 _captureSession 不為 nil 而又重複執行一次 setupCaptureSession 的情況發生。
使用方法抽出，以 isMemoryWarningOccuredAndCaptureSessionCreatedAlready 命名之：
- (BOOL)isMemoryWarningOccuredAndCaptureSessionCreatedAlready
{
return (_captureSession!=nil);
}
- (void)setupCaptureSession
{
if ([self isMemoryWarningOccuredAndCaptureSessionCreatedAlready])
{
return;
}
//以下忽略
}
上面的程式碼中，雖然方法名字很長，不過重構者必須以假設：起碼，日後回來看程式碼的自己要能理解程式的真實動作及其意圖才行，而且長名在編譯後就會消失，所以用意圖明確的名稱絕對值得。此外，即使 if 或其他條件句中的述句只有一句，也最好完整加上大括號。在為數者眾、以及我自己團隊的 coding guideline 中，我都會加上此一要求。
步驟七：持續重構 setupCaptureSession method
可以看到第65行是很長的註解，67~72行是在偵測裝置是否擁有相機。若沒有就離開此 method。一樣，用方法抽出進行重構，抽出 isNoVideoCamera 方法：
步驟8：收攏整理 isMemoryWarningOccuredAndCaptureSessionCreatedAlready 和 isNoVideoCamera
理由：可以看出這兩個方法都是 setupCaptureSession 執行的前置條件，所以可以再進一步收攏，並整理好順序如下。
- (void)setupCaptureSession
{
if ([self isMemoryWarningOccuredAndCaptureSessionCreatedAlready] ||
[self isNoVideoCamera])
{
return;
}
_captureSession = [[AVCaptureSession alloc] init];
//略
}
- (BOOL)isMemoryWarningOccuredAndCaptureSessionCreatedAlready
{
return (_captureSession!=nil);
}
- (BOOL)isNoVideoCamera
{
_videoDevice = [AVCaptureDevice defaultDeviceWithMediaType:AVMediaTypeVideo];
if (_videoDevice == nil) {
NSLog(@"No video camera on this device!");
return YES;
}
return NO;
}
步驟9：
理由：
觀察或實驗一下第64~65行，會發現：
1. 主要是為了建立、設定 _previewLayer。
2. 建立 _previewLayer 時需要傳入 _captureSession 。
其中，「需要傳入 _captureSession 」成為這兩行寫在 setupCaptureSession 方法中的唯一理由。但是 _previewLayer 本身的建立與設定 capture session 並不是絕對需要關聯在一起的同一件事。
方法：
1. 把 64,65行進行方法抽出，並且使其呼叫順序符合需求。(在 setupCaptureSession 方法後再進行呼叫)
步驟10：質疑 running property 的必要性。
- (void)stopRunning {
if (!_running) return;
[_captureSession stopRunning];
_running = NO;
}
- (void)startRunning
{
if (_running)
return;
[_captureSession startRunning];
_metadataOutput.metadataObjectTypes = _metadataOutput.availableMetadataObjectTypes;
_running = YES;
}
理由：
在上面程式碼中， 對 running 這個 property 存在的必要性，感到質疑，因為 captureSession 有一個 instance method : isRunning 應該是相同功能的方法。在單線程運行的 App 裡，像 running 這種 flag 變數的審查不太難。
說明：
1. 註解 running property 並檢視其影響範圍。
2. 以 [_captureSession isRunning] 取代 _running 並移除多餘的程式碼
3. 測試
結果如下：
#pragma mark camera methods
- (void)stopRunning {
if (![_captureSession isRunning])
{
return;
}
[_captureSession stopRunning];
}
- (void)startRunning
{
if ([_captureSession isRunning])
{
return;
}
[_captureSession startRunning];
_metadataOutput.metadataObjectTypes = _metadataOutput.availableMetadataObjectTypes;
}
嗯，應該可以再進一步重構。結果如下：
#pragma mark - camera methods
- (void)captureSessionSwitchON:(BOOL)yes
{
if (yes && ![_captureSession isRunning]) {
[_captureSession startRunning];
} else if ((!yes && [_captureSession isRunning])){
[_captureSession stopRunning];
}
}
當然，原本 stopRunning 及 startRunning 方法的引用也要跟著修改。
步驟11：質疑雙層 enumerateObjectsUsingBlock 的必要
- (void)captureOutput:(AVCaptureOutput *)captureOutput didOutputMetadataObjects:(NSArray *)metadataObjects fromConnection:(AVCaptureConnection *)connection
{
NSMutableSet *foundBarcodes = [[NSMutableSet alloc] init];
[metadataObjects enumerateObjectsUsingBlock:^(AVMetadataObject *obj, NSUInteger idx, BOOL *stop)
{
[metadataObjects enumerateObjectsUsingBlock:^(AVMetadataObject *obj, NSUInteger idx, BOOL *stop) {
NSLog(@"Metadata: %@", obj);
if ([obj isKindOfClass:[AVMetadataMachineReadableCodeObject class]])
{
AVMetadataMachineReadableCodeObject *code = (AVMetadataMachineReadableCodeObject*)[_previewLayer transformedMetadataObjectForMetadataObject:obj];
Barcode *barcode = [self processMetadataObject:code];
[foundBarcodes addObject:barcode];
}
}];
dispatch_sync(dispatch_get_main_queue(), ^{
...(略)
}];
});
}];
說明：
1. 原程式中在 captureOutput:didOutputMetadataObjects:fromConnection 中以雙層巢狀的 enumerateObjectsUsingBlock 呼叫寫成。可以嚐試把內層結構提出與外層結構處於同一層次，並觀察結果是否有異。經過實驗後，我決定把這個巢狀結構移除。
2. 然後將原本內層enumerateObjectsUsingBlock中的程式碼，以方法抽出進行重構。
- (void)findAndBuildBarcodes:(NSMutableSet *)foundBarcodes obj:(AVMetadataObject *)obj
{
if ([obj isKindOfClass:[AVMetadataMachineReadableCodeObject class]])
{
AVMetadataMachineReadableCodeObject *code = (AVMetadataMachineReadableCodeObject*)[_previewLayer transformedMetadataObjectForMetadataObject:obj];
Barcode *barcode = [self processMetadataObject:code];
[foundBarcodes addObject:barcode];
}
}
- (void)captureOutput:(AVCaptureOutput *)captureOutput didOutputMetadataObjects:(NSArray *)metadataObjects fromConnection:(AVCaptureConnection *)connection
{
NSMutableSet *foundBarcodes = [[NSMutableSet alloc] init];
[metadataObjects enumerateObjectsUsingBlock:^(AVMetadataObject *obj, NSUInteger idx, BOOL *stop)
{
[self findAndBuildBarcodes:foundBarcodes obj:obj];
...(略)
});
}];
...(略)
}
重構12：質疑 captureOutput:didOutputMetadataObjects 方法內，NSMutableSet *foundBarcodes 的合理範圍。
原程式：
- (void)captureOutput:(AVCaptureOutput *)captureOutput didOutputMetadataObjects:(NSArray *)metadataObjects fromConnection:(AVCaptureConnection *)connection
{
NSMutableSet *foundBarcodes = [[NSMutableSet alloc] init];
[metadataObjects enumerateObjectsUsingBlock:^(AVMetadataObject *obj, NSUInteger idx, BOOL *stop)
{
...(略)
}];
...(略)
}
決定縮小其範圍，並以 findAndBuildBarcodes 取代 findAndBuildBarcodes:obj。至少，看起來更明白 foundBarcodes 這個 set 只與和它真正有關係的程式碼在一起。(和「…略2」所忽略的程式碼無關)
- (void)captureOutput:(AVCaptureOutput *)captureOutput didOutputMetadataObjects:(NSArray *)metadataObjects fromConnection:(AVCaptureConnection *)connection
{
[metadataObjects enumerateObjectsUsingBlock:^(AVMetadataObject *obj, NSUInteger idx, BOOL *stop)
{
NSMutableSet *foundBarcodes = [self findAndBuildBarcodes:obj];
dispatch_sync(dispatch_get_main_queue(), ^{
...(略)
});
}];
...(略2)
}
重構13：抽出 removeAllPreviewLayers 及 drawNewPreviewLayers
原程式：
dispatch_sync(dispatch_get_main_queue(), ^{
// Remove all old layers
NSArray *allSublayers = [_previewView.layer.sublayers copy];
[allSublayers enumerateObjectsUsingBlock:^(CALayer *layer, NSUInteger idx, BOOL *stop) {
if (layer != _previewLayer) {
[layer removeFromSuperlayer];
}
}];
// Add new layers
[foundBarcodes enumerateObjectsUsingBlock:^(Barcode *barcode, BOOL *stop) {
CAShapeLayer *boundingBoxLayer = [CAShapeLayer new];
boundingBoxLayer.path = barcode.boundingBoxPath.CGPath;
boundingBoxLayer.lineWidth = 2.0f;
boundingBoxLayer.strokeColor = [UIColor greenColor].CGColor;
boundingBoxLayer.fillColor = [UIColor colorWithRed:0.0f green:1.0f blue:0.0f alpha:0.5f].CGColor;
[_previewView.layer addSublayer:boundingBoxLayer];
CAShapeLayer *cornersPathLayer = [CAShapeLayer new];
cornersPathLayer.path = barcode.cornersPath.CGPath;
cornersPathLayer.lineWidth = 2.0f;
cornersPathLayer.strokeColor = [UIColor blueColor].CGColor;
cornersPathLayer.fillColor = [UIColor colorWithRed:0.0f green:0.0f blue:1.0f alpha:0.5f].CGColor;
[_previewView.layer addSublayer:cornersPathLayer];
}];
重構後：
dispatch_sync(dispatch_get_main_queue(), ^{
[self removeAllPreviewLayers];
[self drawNewPreviewLayers:foundBarcodes];
});
- (void)removeAllPreviewLayers
{
NSArray *allSublayers = [_previewView.layer.sublayers copy];
[allSublayers enumerateObjectsUsingBlock:^(CALayer *layer, NSUInteger idx, BOOL *stop) {
if (layer != _previewLayer) {
[layer removeFromSuperlayer];
}
}];
}
- (void)drawNewPreviewLayers:(NSMutableSet *)foundBarcodes
{
[foundBarcodes enumerateObjectsUsingBlock:^(Barcode *barcode, BOOL *stop) {
CAShapeLayer *boundingBoxLayer = [CAShapeLayer new];
boundingBoxLayer.path = barcode.boundingBoxPath.CGPath;
boundingBoxLayer.lineWidth = 2.0f;
boundingBoxLayer.strokeColor = [UIColor greenColor].CGColor;
boundingBoxLayer.fillColor = [UIColor colorWithRed:0.0f green:1.0f blue:0.0f alpha:0.5f].CGColor;
[_previewView.layer addSublayer:boundingBoxLayer];
CAShapeLayer *cornersPathLayer = [CAShapeLayer new];
cornersPathLayer.path = barcode.cornersPath.CGPath;
cornersPathLayer.lineWidth = 2.0f;
cornersPathLayer.strokeColor = [UIColor blueColor].CGColor;
cornersPathLayer.fillColor = [UIColor colorWithRed:0.0f green:0.0f blue:1.0f alpha:0.5f].CGColor;
[_previewView.layer addSublayer:cornersPathLayer];
}];
}
原程式：
- (Barcode *)processMetadataObject:(AVMetadataMachineReadableCodeObject*)code {
// 1 Query the dictionary of Barcode objects to see if a Barcode with the same contents is already cached.
Barcode *barcode = [self findOrCreateNewBarcode:code];
// Create the path joining code's corners
// 4 Instantiate cornersPath to store the path joining the four corners of the code.
CGMutablePathRef cornersPath = CGPathCreateMutable();
// 5 Convert the first corner coordinate to CGPoint instances using some CoreGraphics calls.
CGPoint point;
CGPointMakeWithDictionaryRepresentation((CFDictionaryRef)code.corners[0], &point);
// 6 Begin the path at the corner defined in Step 5.
CGPathMoveToPoint(cornersPath, nil, point.x, point.y);
// 7 Loop through the other three corners, creating the path as you go.
for (int i = 1; i < code.corners.count; i++) {
CGPointMakeWithDictionaryRepresentation((CFDictionaryRef)code.corners[i], &point);
CGPathAddLineToPoint(cornersPath, nil, point.x, point.y);
}
// 8 Close the path by joining the fourth point to the first point.
CGPathCloseSubpath(cornersPath);
// 9 Create a UIBezierPath object from cornersPath and store it in the Barcode object
barcode.cornersPath = [UIBezierPath bezierPathWithCGPath:cornersPath];
CGPathRelease(cornersPath);
// Create the path for the code's bounding box
// 10 Create the bounding box path using bezierPathWithRect:.
barcode.boundingBoxPath = [UIBezierPath bezierPathWithRect:code.bounds];
// 11 Finally, return the Barcode object.
return barcode;
}
重構後：
- (Barcode *)processMetadataObject:(AVMetadataMachineReadableCodeObject*)code {
Barcode *barcode = [self findOrCreateNewBarcode:code];
CGMutablePathRef cornersPath = [self creatBarcodeCornerPathes:code];
[self setupBarcodePreviewPath:cornersPath barcode:barcode code:code];
return barcode;
}
- (Barcode *)findOrCreateNewBarcode:(AVMetadataMachineReadableCodeObject *)code {
Barcode *barcode = _barcodes[code.stringValue];
if (barcode == nil) {
barcode = [Barcode new];
_barcodes[code.stringValue] = barcode;
}
barcode.metadataObject = code;
return barcode;
}
- (CGMutablePathRef)creatBarcodeCornerPathes:(AVMetadataMachineReadableCodeObject *)code {
CGPoint point;
CGMutablePathRef cornersPath = [self creatPathToFirstCorner:&point code:code];
[self buildPathesWithAllCorners:point cornersPath:cornersPath code:code];
CGPathCloseSubpath(cornersPath);
return cornersPath;
}
- (CGMutablePathRef)creatPathToFirstCorner:(CGPoint *)point_p code:(AVMetadataMachineReadableCodeObject *)code {
CGMutablePathRef cornersPath = CGPathCreateMutable();
CGPointMakeWithDictionaryRepresentation((CFDictionaryRef)code.corners[0], &(*point_p));
CGPathMoveToPoint(cornersPath, nil, point_p->x, point_p->y);
return cornersPath;
}
- (void)buildPathesWithAllCorners:(CGPoint)point cornersPath:(CGMutablePathRef)cornersPath code:(AVMetadataMachineReadableCodeObject *)code {
for (int i = 1; i < code.corners.count; i++) {
CGPointMakeWithDictionaryRepresentation((CFDictionaryRef)code.corners[i], &point);
CGPathAddLineToPoint(cornersPath, nil, point.x, point.y);
}
}
- (void)setupBarcodePreviewPath:(CGMutablePathRef)cornersPath
barcode:(Barcode *)barcode
code:(AVMetadataMachineReadableCodeObject *)code {
barcode.cornersPath = [UIBezierPath bezierPathWithCGPath:cornersPath];
CGPathRelease(cornersPath);
barcode.boundingBoxPath = [UIBezierPath bezierPathWithRect:code.bounds];
}
結語：
本範例主要在表達一些重構的觀念及技巧，其實在一個步驟內就還包含了數個小步驟，並不像本文中講的那麼簡潔。假若讀者有一起跟著操作，一定知道我在說什麼。
所有的程式碼均集中在 ViewController.m 中，所以這樣的重構結果，算是很基礎的。不過，這卻是重要的基礎。假設，未來有任何功能性需求的擴充，或是希望將這份程式碼延伸成樣版、模組，或是做為其他應用的基礎，相信妥善重構後的結果，一定能讓上述種種變更需求都顯得更優雅、更可行。至少，對於撰寫者而言，應該能保持較佳的工作心情及效率。
最後注意到的一點是：這個範例，正好是很不容易進行 TDD 的範例。至少目前我還沒找到可以把 UIImage 以 AVMetadataMachineReableCodeObject 進行解析的方式。不過儘管如此，在每一個步驟之間，重構者都必須得要不斷進行測試，並確定功能無損後才進行 commit，這個紀律是非常重要的。
