遏制类规模的膨胀
简洁是天才的姊妹
——契诃夫
上帝啊,别让我看到超过300行代码的类
人类总是会在一些谁都明白不能违背的规则上犯错误。比如,闯红绿灯。谁都明白闯红绿灯可能出现的严重后果,可还是经常有人犯这样的错误。在软件的设计和编码过程中,让一个类的规模无限的膨胀,却是我们最最经常犯下的错误。
正是由于我们所犯下的过错,使得我们在代码的review和维护的过程中,经常会做出这样的祈祷:上帝啊,别让我看到超过300行代码的类。而更为可笑的是,我们一边在做这样的祈祷,一边却在编码的过程中,不断的犯这样错误。
类的过分膨胀,其实首先就违反了面向对象的一个基本原则——单一职责原则:就一个类而言,应该仅有一个引起它的变化的原因。这句话比较难以理解,换句话说,一个类只实现它固有的职责,不是它固有的职责不要强加在它里面。所有在规模上过分膨胀的类,都违反了这一原则。
下面我们来看一看我们是怎么导致我们的类在规模上迅速的膨胀的呢?
第一、 把别的类的功能强加到类中
这是我们对类的功能分析不够所导致的。一句话说,我们在分析和设计的过程中,工作还没有做好,需要我们做进一步的分析和设计。
在软件的分析、设计和编码过程中,由于我们分析和设计的不够细致,经常把一些似是而非的功能强加在一个类中,由于你再作仔细的分析的话,会发现这些功能根本不属于这个类。可想而知,这样的类会引起很多的麻烦。
例如,现在我们有一个Person类,我们在分析的过程中,得出了它拥有这样一些的功能:eat,say,sleep和fly。现在编码如下:
public class Person
{
public void eat()
{
System.out.println(“He is eating something……”);
}
public void say()
{
System.out.println(“He is saying something……”);
}
public void sleep()
{
System.out.println(“He is sleeping…..”);
}
public void fly()
{
System.out.println(“He is flying by airplane……”);
}
}
我们看到了最后一个方法:fly()。马上就会感觉怪怪的,人怎么能飞啊?类的设计者振振有词地说,人当然能飞啊,没看到我在方法中的说明吗?他是坐飞机飞的。于是,我们可能会哑口无言,也对啊,人坐了飞机也能飞。
很多时候,我们的感觉是对的。这个类的设计是有问题的,问题就在于他把别的类的功能强加在这个类上面。我们稍微仔细的分析一下,就会发现fly()是Airplane类的功能,放在Person类里头,真正的让Person类越庖代俎了。
这样做的害处是显而易见的:当系统的功能进行扩展的时候,马上就会发现现有的系统已经扩展不下去。例如,当你的Person类完成不久,系统有了新的需求,要求知道人所座飞机的航班号、价格、时间等等。我们就会发现,我们不得不增加新的类:Airplane。这还不是我们害怕的,真正让我们头疼的是我们的Person类中的fly()方法的功能需要移到Airplane类中去,而Person类中需要做新的方法来访问Airplane,以获得Person类是否乘坐过飞机的信息。这样的改动也就太大了,对我们的Person类已经造成了严重的伤害。
仔细的分析和设计吧,可能我们的一个不经意的错误,就会在我们的系统的扩展的时候带来无法挽回的损失。
记住吧:一个类只实现它的核心功能。不是它的功能请不要强加在它身上。类的功能越简单,它的编码、测试以及维护就越简单。何乐而不为呢?
第二、 把本该由工具类实现的功能强加到了一个特定功能的基础类里
这真是带有迷惑性的类啊,这个类本身是一个基础类,很多类是它的子类。这样的基础类使得你以为可以把它的子类的一些相似的功能放在它里面实现,然后在子类里调用。
但是,不要忘了,这个基础类往往是有特定功能的。不要把它当成一个大杂烩。
public abstract class BaseAction extends Action
{
private static ApplicationContext appCtx = null;
protected final static Logger logger = Logger.getInstance();
protected ParamModel paramModel = null;
protected int permission=AdminConstants.NONE;
static
{
logger.debug("========================= load ApplicationContext....");
String[] contextFiles = null;
if(contextFiles == null)
{
contextFiles = new String[]{"applicationContext.xml",
"dataAccessContext-local.xml"};
}
appCtx = new ClassPathXmlApplicationContext(contextFiles);
}
public BaseAction()
{
}
public abstract ActionForward actionExecute(ActionMapping mapping,
ActionForm form,
HttpServletRequest request,
HttpServletResponse response)
throws Exception;
public ActionForward processSession(ActionMapping mapping, HttpServletRequest request)
{
String username = this.getUsername(request);
if( request.getSession() == null || username == null || username.equals(""))
{
return mapping.findForward(CommonConstants.FORWARD_LOGIN);
}
return null;
}
public ActionForward processPermissionDetect(ActionMapping mapping, HttpServletRequest request) throws Exception
{
//get moduleId and permission needed to detecting privilage
String moduleId = ((PSAActionMapping)mapping).getModuleId();
String userId = this.getUserId(request);
int mId = StringUtils.strToInt(moduleId);
int uId = StringUtils.strToInt(userId);
if(uId == AdminConstants.ADMINISTRATOR_ID)
{
permission = AdminConstants.EDIT;
}
else
{
//if(moduleId != null) should be ignore after debugging
if(moduleId != null){//if the moduleId property had been set,the relative user permission must be detected
permission = ((AdminService)getAdminService()).getPermission(uId,mId);
if(permission == AdminConstants.NONE)
{
return mapping.findForward(CommonConstants.FORWARD_LOGIN);
}
}
}
if(moduleId != null)
{
request.setAttribute(AdminConstants.MODULE_KEY_IN_REQUEST,moduleId);
}
return null;
}
public final ActionForward execute(ActionMapping mapping, ActionForm form,
HttpServletRequest request,
HttpServletResponse response)
throws Exception
{
// System.out.println(this.getClass().getName());
boolean bListTree = false;
if ( "ListTreeAction".equals(this.getClass().getName()) )
{
bListTree = true;
}
else
{
ActionForward af = processSession(mapping, request);
if( af != null)
{
return af;
}
af = processPermissionDetect(mapping, request);
if( af != null)
{
return af;
}
paramModel = (ParamModel)request.getSession().getAttribute(CommonConstants.SESSION_PARAM);
if(request.getSession().getAttribute("CommonService") == null)
{
request.getSession().setAttribute("CommonService", getCommonService());
}
}
//do the special action
ActionForward forward = actionExecute(mapping, form, request, response);
//do after the action
//get the tab data from database and set session of some
// middle vars
if ( ! bListTree )
{
String servletPath = request.getServletPath();
//if action path begins with "/working/", set page navigation information into request
if(servletPath.indexOf("/" + CommonConstants.ACTION_PATH_PREFIX + "/") == 0)
{
request.setAttribute(CommonConstants.PAGE_NAVIGATION, this.getPageNav(forward.getName()));
}
else
{
setTabData(request);
setLocationWizard( request, forward.getName());
}
}
return forward;
}
private String getPageNav(String forwardName)
{
String compondKey = this.getClass().getName() + "_" + forwardName;
FreebordersProperties fbProperties = new FreebordersProperties("locationWizard.properties");
String commandValue = fbProperties.getProperty(compondKey);
MessageResources messages = MessageResources.getMessageResources("commonRS");
String result = messages.getMessage(commandValue);
return result;
}
private void setLocationWizard(HttpServletRequest request, String forwardString)
{
……
}
private void setTabData(HttpServletRequest request) throws InvalidOrderMapException
{
{
Options[] keys = mapTab.optionsKeys();
OrderMap newMap = new OrderMap();
for(int i=0;i
{
newMap.put(keys[i], (String)mapTab.get(keys[i]) + "&nodeId=" + nodeId);
}
mapTab = newMap;
}
strTabData = tabHelper.getTabData( strWebPath, mapTab, strSelected);
}
request.setAttribute("TabData",strTabData);
}
public static Object getPlanService()
{
return appCtx.getBean("planService");
}
public static Object getTaskService()
{
return appCtx.getBean("taskService");
}
public static Object getDictService()
{
return appCtx.getBean("dictService");
}
……
public static Object getOtherMetricsService()
{
return appCtx.getBean("otherMetricsService");
}
public List getLb(long ld)
{
List lst = null;
try
{
DictService dt = (DictService)getDictService();
lst = (List)dt.queryDictByType(ld);
DictCategoryValueModel model = new DictCategoryValueModel();
model.setCategoryValueId(0);
model.setCategoryCode("");
lst.add(0,model);
}
catch(Exception e)
{
e.printStackTrace();
}
return lst;
}
public List getLb(long ld, long obj)
{
List lst = null;
try
{
DictService dt = (DictService)getDictService();
lst = (List)dt.queryDictByType(ld, obj);
DictCategoryValueModel model = new DictCategoryValueModel();
model.setCategoryValueId(0);
model.setCategoryCode("");
lst.add(0,model);
}
catch(Exception e)
{
e.printStackTrace();
}
return lst;
}
public String getConditionSql(ConditionForm form)
{
String ls = "";
String columnname = StringUtils.strObjectToString(form.getColumnName()).trim();
String operator = StringUtils.strObjectToString(form.getOperator()).trim();
String value = StringUtils.strObjectToString(form.getValue()).trim();
value = value.replaceAll("'","''");
if(columnname.equals("")||operator.equals(""))
{
return ls;
}
String colum[] = columnname.split("~~");
if(colum[1].equals(CommonConstants.COLUMN_STRING))
{
return getStringSql(colum[0],operator,value);
}
if(colum[1].equals(CommonConstants.COLUMN_NUMBER))
{
return getNumberSql(colum[0],operator,value);
}
if(colum[1].equals(CommonConstants.COLUMN_DATE))
{
return getDateSql(colum[0],operator,value);
}
if(colum[1].equals(CommonConstants.COLUMN_DIC))
{
return getDicSql(colum[0],operator,value);
}
if(colum[1].equals(CommonConstants.COLUMN_SPECIAL))
{
return getSpecialSql(colum[0],operator,value);
}
return ls;
}
public String getStringSql(String column,String operator,String value)
{
String ls = "";
if(!value.equals(""))
{
if(operator.equals(CommonConstants.OPERATOR_LIKE))
{
ls+=column + " " + operator + " " + "'%"+value+"%'";
}
else if(operator.equals(CommonConstants.OPERATOR_NO_EQUAL))
{
ls+=column + " " + operator + " " + "'"+value+"'";
ls+=",,"+column+ " is null";
}
else
{
ls+=column + " " + operator + " " + "'"+value+"'";
}
}
else
{
if(operator.equals(CommonConstants.OPERATOR_NO_EQUAL))
{
ls+=column + " is not null";
}
else
{
ls+=column + " is null";
}
}
return ls;
}
……
public HashMap AddMap(String col,String oper)
{
HashMap map = new HashMap();
map.put("id", col);
map.put("name",oper);
return map;
}
……
private String truncateStr(String strWiardValue, String endStr)
{
strWiardValue = strWiardValue.trim();
if(strWiardValue.endsWith(endStr))
{
strWiardValue = strWiardValue.substring(0, strWiardValue.length() - endStr.length());
strWiardValue = strWiardValue.trim();
}
return strWiardValue;
}
……
}
这段代码一共有1032行代码,在被我省略掉很多行的代码以后,依然占据了好几个页面。
其实,这个类的功能很简单。就是要把所有的Action类的一些基本的、共有的功能实现在这个BaseAction中,然后各个Action类都继承它,自然就实现了这个基本的功能。它使用了模板方法模式,把基本的功能实现在这个类里,然后使用一个纯虚方法:actionExecute,将各个Action类的不同功能预留到这个方法里面去实现。
这本来是一个很好的基础类,但是在慢慢的编码过程中,它渐渐的发臭了。原因就在于一些开发人员没有弄清楚这个类的功能:这是一个基础类,用来实现所有Action的一些共同的功能。而开发人员却把它当成了一个工具类,把所有的需要在Helper类实现的功能都放在了里面。从而使得代码不断的膨胀,慢慢的发臭。
首先要分析清楚,模块的基础类和工具类的区别:模块的基础类是要被本模块使用的,其他的模块不太可能使用到,其他的系统也不可能使用到。如上面的BaseAction类。很明显,这个类只在action模块中被使用到,而在business层和data层不会用到。所以它里面要实现的功能也仅限于action模块中的一些公共功能。而工具类是会被很多模块公用的一些功能,很多时候会被不同的系统复用。最基本的例子是一些对字符串的操作功能,几乎所有的模块都会用到这些功能;而且其他的系统也会使用到。
我们再来看上面的BaseAction类。很明显,setTabData方法实现的功能是action模块独有的,它用来对每一个页面上的Tab进行设置,这是每一个Action必须完成的一个功能。所以这个setTabData方法放在BaseAction类里是允许的,也是正当的。
我们再往下看,两个getLb方法其实实现的是业务层的功能,放在这里就已经欠妥了,因为如果业务一旦发生时,还要到表现层的action类中来做一定的修改,这就违背了分层的原则。
再往下来,getConditionSql方法和getStringSql方法放在这里就太不应该了。很明显,这是对sql语句的一些处理方法。除了这两个方法,我还省略了好几个同样是处理sql语句的方法。首先,我现在不想追究在表现层处理sql语句是否正确,我们的第一想象应该是在data层来处理,这里我不想讨论这个问题,因为你可能有很多种理由在这里处理你的sql语句。但是把这些处理sql的方法放在这里,实在是有太多太多的问题。其实这些sql语句的方法可以被归纳到工具类里去。为什么这么说呢?首先在data层我们极有可能会使用到这些方法,那么如果我们在data层想使用这些方法,就不得不到表现层来调用这些方法。这严重造成了data层和表现层之间的依赖关系,要知道,这两个层最起码隔了一个业务层,是不可能直接产生依赖关系的。其次,这些方法很有可能会被其他系统的data层使用,这样的话,难道我们要把整个BaseAction类移植到别的系统不成?
要解决上面的问题很简单,在系统的util包里做一个SqlHelper类,然后把这些sql语句的功能都移植到这个类里。这样,不管是data层调用,还是表现层调用,都不存在问题。更为重要的是,如果我们在一个新的系统中需要使用到这个sql语句的功能,那么将util包移植到那个系统中去就可以了。
我们还可以往下看,仍然可以找到很多这样的错误,在这里我就不再多说了。
第三、 功能过于庞大
有些时候,某个类的功能看似实现的是它的一些固有功能。但由于这个类或方法实现的功能过于庞大,所以需要进行拆分,以方便我们的分析、设计和编码的实现,同时,对于每一个这样短小精悍的功能,在测试和维护上,我们也很容易。
下面我们来看一个功能过分庞大的类的例子:
public class ToSpeAction extends Action {
public ActionForward execute(ActionMapping actionMapping, ActionForm actionForm, HttpServletRequest request, HttpServletResponse response)
{
……
String replacePomStr = TransformTools.getObjectString(request.getParameter("replacePomStr"));
String sizeChangeFlagForChange = TransformTools.getObjectString(request.getParameter("sizeChangeFlagForChange"));
String sampleSelect= TransformTools.getObjectString(request.getParameter("sampleSelect"));//for sample size value changer
……
System.out.println("the pomId"+pomId);
DynaActionForm form = (DynaActionForm)actionForm;
System.out.println("(String)form.get(txtDesignID)1="+(String)form.get("txtDesignID"));
//get Intial Data
getIntialData( request);
SpeciManager speciManager=new SpeciManager();
String action = TransformTools.getObjectString(request.getParameter("action"),"load");
if(action.equals("load")&&tab.equals("0")){
request.setAttribute("tabId",tab);
form.set("tabId",tab);
speciManager.loadSumary( actionForm, request, tab);
}
if(action.equals("Save")&&tab.equals("0")){
……
if(!sortPomStr.equals(""))
speciManager.saveSort( request,(String)request.getSession().getAttribute("specId"),sortPomStr);
if(!addPomStr.equals(""))
speciManager.saveAddPom( request,(String)request.getSession().getAttribute("specId"),GradeRuleIdforPom,addPomStr);
//size change settle
……
}
}else{
……
request.setAttribute("tabId",tab);
}
if(action.equals("load")&&tab.equals("1")){
request.setAttribute("tabId",tab);
form.set("tabId",tab);
speciManager.loadIncrement(actionForm, request, tab);
}
if(action.equals("Save")&&tab.equals("1")){
System.out.println("inter increment save action");
……
}
}else{
……
request.setAttribute("tabId",tab);
}
if(action.equals("load")&&tab.equals("2")){
request.setAttribute("tabId",tab);
form.set("tabId",tab);
speciManager.loadMeasureMent(actionForm, request, tab);
}
if(action.equals("Save")&&tab.equals("2")){
System.out.println("inter increment save action");
……
}
……
}
///because no data ,in order to
……
}
// get Initail data
public void getIntialData( HttpServletRequest request){
IHashMap map = new IHashMap();
InitHeadData PBean = new InitHeadData();