1、 清晰地表达意图
2、 一个方法只做一件事情
3、 同一个方法体内，保持相同的抽象层次
4、 不要重复自己（避免手动的复制与粘贴代码）
5、 减少“语法噪音”
6、 命名时取有意义的名字，避免不规范的缩写
public DataSet GetDefectDetails(string sDefGrp)
{
bool bResult =false;
DataSet dsData =new DataSet();
try
{
Hashtable htFilters = new Hashtable();
htFilters.Add("GROUP_NAME", sDefGrp);
bResult = fisService.GetDataSet(QueryName.GET_CODE_BY_GROUP_NAME, htFilters,ref dsData);
}
catch(Exception Ex)
{
string sErrorMessage = Helper.GetErrorDescription(ex.Message);
if (sErrorMessage== string.Empty)
{
sErrorMessage = ex.Message;
}
Master.ErrorMessage = sErrorMessage;
m_logMsg.Message = "GetDefectDetails : " +sErrorMessage;
m_logger.LogMsg(m_logMsg);
}
return dsData;
}
我们来看看，这20行代码，真正“在干活”的代码有哪些？
看来看去，只有这么一行：
bResult =fisService.GetDataSet(QueryName.GET_CODE_BY_GROUP_NAME, htFilters,ref dsData);
我们可以说，其他的代码，其实都算是“语法噪音”。当然，由于我们不是生活在真空中，所以有些语法噪音是没办法完全避免的，我们能做的只是“减少”，使之不至于影响我们意图的表达。
但是，单单分析这个语句，我们也能发现一些“噪音”，这里将fisService.GetDataSet的结果赋给变量bResult，如果不看完整个方法，我们会假定这个bResult就是这个GetDefectDetails方法的返回值，但看到最后我们惊讶地发现，返回值是dsData，而这个bResult居然没用到。
可以推断，这就是复制粘贴代码惹的祸，在原来复制代码的地方，bResult应该就是方法的返回值，在copy过来后，由于这个变量不影响执行的过程，就被保留下来了。我们前面说过，代码有双重目的，copy代码的人显然忽略了第2个目的：代码的可读性。
OK，我们修改一下 ，这段代码可以变成19行，而且我们也不用再关心那个奇怪的变量bResult：
public DataSet GetDefectDetails(stringsDefGrp)
{
DataSet dsData =new DataSet();
try
{
HashtablehtFilters = new Hashtable();
htFilters.Add("GROUP_NAME", sDefGrp);
fisService.GetDataSet(QueryName.GET_CODE_BY_GROUP_NAME, htFilters,ref dsData);
}
catch(Exception Ex)
{
stringsErrorMessage = Helper.GetErrorDescription(ex.Message);
if (sErrorMessage== string.Empty)
{
sErrorMessage = ex.Message;
}
Master.ErrorMessage = sErrorMessage;
m_logMsg.Message = "GetDefectDetails : " +sErrorMessage;
m_logger.LogMsg(m_logMsg);
}
return dsData;
}
有同学说，这个改善太微不足道了。对的，重构的精神就是小步推进，勿因善小而不为，通过不断积累，最后就可以得到一个清晰、易维护的系统。
然后，我们的目光移向那个可疑的if语句，为什么要做这个判断？或者说，这个判断能不能放到Helper.GetErrorDescription方法内部？通过阅读Helper.GetErrorDescription方法的代码，我们发现这个判断完全可以方法内部。
在这里，造成重复代码的原因是：写共用方法的人未考虑周全，导致客户需要关心方法的条件分支情况，从外部进行判断。
通过将判断逻辑移到Helper.GetErrorDescription方法内部，代码变成15行。
public DataSet GetDefectDetails(stringsDefGrp)
{
DataSet dsData =new DataSet();
try
{
Hashtable htFilters = new Hashtable();
htFilters.Add("GROUP_NAME", sDefGrp);
fisService.GetDataSet(QueryName.GET_CODE_BY_GROUP_NAME, htFilters,ref dsData);
}
catch(Exception Ex)
{
strings ErrorMessage = Helper.GetErrorDescription(ex.Message);
Master.ErrorMessage = sErrorMessage;
m_logMsg.Message = "GetDefectDetails : " +sErrorMessage;
m_logger.LogMsg(m_logMsg);
}
return dsData;
}
现在再看看，try…catch…的语句块还是占据了大部分的空间，而且，每个异常处理的过程都是一样的，这里也会导致大量的手动复制粘贴代码，难道这部分代码不能只写一次吗？我们重新审视一下代码中变化的地方，用高亮显示出来：
try
{
Hashtable htFilters = new Hashtable();
htFilters.Add("GROUP_NAME", sDefGrp);
fisService.GetDataSet(QueryName.GET_CODE_BY_GROUP_NAME, htFilters,ref dsData);
}
catch(Exception Ex)
{
strings ErrorMessage = Helper.GetErrorDescription(ex.Message);
Master.ErrorMessage = sErrorMessage;
m_logMsg.Message = "GetDefectDetails : " +sErrorMessage;
m_logger.LogMsg(m_logMsg);
}
OK，看来我们需要一个模板方法，第1个高亮的语句我们可以定义成一个委托，第2个高亮的语句是其实是方法的名称。我们可以在基类中定义：
protected delegate void MyAction();
protected void HandleException(MyAction myAction)
{
try
{
myAction();
}
catch(Exception Ex)
{
stringsErrorMessage = Helper.GetErrorDescription(Ex.Message);
Label lblMsg =(Label)Master.FindControl("lblMsg");
if(lblMsg != null)
{
lblMsg.Text = sErrorMessage;
lblMsg.CssClass = "Err";
}
mylogMsg.Message = myAction.Method.Name + " : " +sErrorMessage;
mylogger.LogMsg(mylogMsg);
}
}
好了，这回我们连处理异常的细节都不用关心了。由于.net2.0可以写匿名方法，代码可以写得很简洁，新的写法只剩下8行：
public DataSet GetDefectDetails(stringsDefGrp)
{
DataSet dsData =new DataSet();
HandleException(delegate()
{
Hashtable htFilters = new Hashtable();
htFilters.Add("GROUP_NAME", sDefGrp);
fisService.GetDataSet(QueryName.GET_CODE_BY_GROUP_NAME, htFilters,ref dsData);
});
return dsData;
}
好了，现在看看我们的成果：
1、 把代码从20行简化到8行（减少语法噪音）
2、 将重复的逻辑放到公用方法中（避免手动的复制粘贴代码）
3、 需要关注的地方大大减小（不用再关心bResult变量和异常处理细节，更专注于业务逻辑）
在现有框架的基础上，这已经算是一个极致，下面，让我们作一个小小的冒险，假设框架可以由我们自由定义，看看我们可以重构到什么地步。
首先，通过AOP(aspect-orientedprogramming)框架，我们可以让异常处理语句完全在方法体内消失，只需要在处理异常的方法上打上一个标签：
[HandleException]
public DataSet GetDefectDetails(stringsDefGrp)
{
DataSet dsData =new DataSet();
Hashtable htFilters = new Hashtable();
htFilters.Add("GROUP_NAME", sDefGrp);
fisService.GetDataSet(QueryName.GET_CODE_BY_GROUP_NAME, htFilters,refdsData);
return dsData;
}
这里体现了“声明式编程”的风格，即只要说明意图，而不需要写出处理细节。
当然以上仅为示意写法，HandleException的attribute必须另外定义，使用不同的AOP框架，写法也有不同，常用的AOP框架有Sprint.NET, Castle,PostSharp等。
剩下最大的“噪音”应该就是这个Hashtable了。在这里定义Hashtable，仅仅是框架的需要，同样，QueryName也是框架的需要，如果按“正常的写法”，代码应该是：
[HandleException]
public DataSet GetDefectDetails(stringsDefGrp)
{
return fisService.GetCodeByGroupName(sDefGrp);
}
OK，这回这个方法终于露出本来面目了，GetDefectDetails方法根本没有处理任何业务逻辑，之所以有20行的代码，只是因为框架本身不够完善，无法帮助程序员简化开发而已，所以…其实…这个方法…可以拿掉…
GetDefectDetails方法：哦，不！你不是来帮我改进的吗？怎么把我给杀掉了？
背景声音：不好意思，我国的改造，历来如此…
余论：
1、 问：如果方法删掉了，在哪里注明需要异常处理？
答：可以在调用GetDefectDetails方法的方法上打上HandleException的attribute，或者在配置文件中指定webForm中所有的异常使用统一的方法处理。
2、 其实我理想中的方法签名应该是：
Class CodeService…
public List<CodeInfo> GetCodeInfosByGroupName(stringcodeGroupName)
修改的地方：
a、使用泛型的实体对象列表替换掉DataSet；
b、对于类型很明显的对象，不必加上类型前缀(此处sDefGrp用s代表string类型)，以减少语法噪音。我觉得只有控件名称（如：lblMessage）、“容器”类型（如：htFilters）需要类型前缀加以说明。
c、不使用缩写（def和grp都不能算是规范而通用的缩写。）