《重构-改善既有代码设计》案例之C#版(2)
书接上文。。。
先来看一眼这个AmountFor方法
1 private int AmountFor(Performance aPerformance, Play play) 2 { 3 int result; 4 switch (play.type) 5 { 6 case "tragedy": 7 result = 40000; 8 if (aPerformance.audience > 30) 9 { 10 result += 1000 * (aPerformance.audience - 30); 11 } 12 13 break; 14 case "comedy": 15 result = 30000; 16 if (aPerformance.audience > 20) 17 { 18 result += 1000 + 500 * (aPerformance.audience - 20); 19 } 20 21 break; 22 default: 23 throw new Exception($"unknown type:{play.type}"); 24 } 25 26 return result; 27 }
直觉上这个参数是不是有点问题...按理说演出Performance应该是带有剧目Play信息的(至少带有playid,我们可以添加一个方法通过这个playid获取),不需要再额外传一个play给方法。所以我们去掉这个play参数.这里用到一个重构手法:以查询替代临时变量
1 private Play PlayFor(Performance aPerformance) 2 { 3 return _plays[aPerformance.playID]; 4 }
所以AmoutFor成了这样
1 private int AmountFor(Performance aPerformance) 2 { 3 int result; 4 switch (PlayFor(aPerformance).type) 5 { 6 case "tragedy": 7 result = 40000; 8 if (aPerformance.audience > 30) 9 { 10 result += 1000 * (aPerformance.audience - 30); 11 } 12 13 break; 14 case "comedy": 15 result = 30000; 16 if (aPerformance.audience > 20) 17 { 18 result += 1000 + 500 * (aPerformance.audience - 20); 19 } 20 21 break; 22 default: 23 throw new Exception($"unknown type:{PlayFor(aPerformance).type}"); 24 } 25 26 return result; 27 }
调用的地方也成了这样
int thisAmount = AmountFor(perf);
再想一想,在Statement方法内部thisAmount是不是可以用AmountFor方法替代,play也可以用PlayFor方法替代.不需要这两个变量了.在需要的地方直接用这两个方法替换.这个重构方法叫:内联变量
所以代码编程这样
1 public string Statement() 2 { 3 int totalAmount = 0; 4 int volumeCredits = 0; 5 string result = $"Statement for {_invoice.Customer} \n"; 6 NumberFormatInfo nfi = new CultureInfo("en-US").NumberFormat; 7 nfi.CurrencyDecimalDigits = 2; 8 9 foreach (var perf in _invoice.performances) 10 { 11 //add volume credits 12 volumeCredits += Math.Max(perf.audience - 30, 0); 13 //add extra credit for every ten comedy attendees 14 if ("comedy" == PlayFor(perf).type) 15 { 16 volumeCredits += perf.audience / 5; 17 } 18 19 //print line for this order 20 result += $"{PlayFor(perf).name}: {string.Format(nfi, "{0:C}", AmountFor(perf) / 100)}({perf.audience}seats)\n"; 21 totalAmount += AmountFor(perf); 22 } 23 24 result += $"Amount owed is {string.Format(nfi, "{0:C}", totalAmount / 100)}\n"; 25 result += $"You earned {volumeCredits} credits \n"; 26 return result; 27 }
现在这个Statement方法是不是已经比原始版本看上去短多啦!foreach里面就干了三件事,算积分,拼接输出字符串,算总金额。算积分的逻辑在这里显的格格不入
接下来,还要做一件事,就是把处理积分的逻辑分离出来,单独提炼出一个方法
1 private int VolumeCreditsFor(Performance perf) 2 { 3 int result = Math.Max(perf.audience - 30, 0); 4 if ("comedy" == PlayFor(perf).type) 5 { 6 result += perf.audience / 5; 7 } 8 9 return result; 10 }
所以现在看foreach里面直接三个+=是不是统一和谐了
1 public string Statement() 2 { 3 int totalAmount = 0; 4 int volumeCredits = 0; 5 string result = $"Statement for {_invoice.Customer} \n"; 6 NumberFormatInfo nfi = new CultureInfo("en-US").NumberFormat; 7 nfi.CurrencyDecimalDigits = 2; 8 9 foreach (var perf in _invoice.performances) 10 { 11 volumeCredits += VolumeCreditsFor(perf); 12 //print line for this order 13 result += $"{PlayFor(perf).name}: {string.Format(nfi, "{0:C}", AmountFor(perf) / 100)}({perf.audience}seats)\n"; 14 totalAmount += AmountFor(perf); 15 } 16 17 result += $"Amount owed is {string.Format(nfi, "{0:C}", totalAmount / 100)}\n"; 18 result += $"You earned {volumeCredits} credits \n"; 19 return result; 20 }
本篇完成...待续....