遏制类规模的膨胀
作者 软件的信雅达
简洁是天才的姊妹
——契
诃
夫
上帝啊，别让我看到超过
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<keys.length;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();
PBean.getInitData( map , TransformTools.getObjectString( request.getSession().getAttribute("DivisionStr")) ) ;
String[] keys = map.keys() ;
for(int i = 0; i < keys.length;i++)
{
request.setAttribute(keys[i],map.get(keys[i]) ) ;
}
}
}
这个类的整个规模大约在
340
行代码左右，似乎臭味不是很重。而且它实现的功能也是相当的内聚。一共实现了两个相似的页面，在代码分别用
tab=1
和
tab=2
来标识；每一个页面都有三个功能：
load
、
save
和
refresh
。算起来，在这个类的一个方法里面，相当于实现了
6
个功能。首先的想法是这个类的方法所实现的功能是不是太多了一点。类的设计者明确地说，这些功能有很多共同的地方，也有相当多的不同的地方。把它们集中在一个类的方法里面，有利于重用它们的相同之处。
他的说法是不错的。把它们写在一起的确有利于重用，但是却带来了更大的麻烦。首先是这么多
if…else if…
在一个方法里，容易造成思路的混乱，你可能会把这个功能写到了那个功能模块里面，那个功能写到了这个功能模块里面，造成了相互的混乱。事实上的确如此，类的设计者最终也没能完成这个类。而在我们接手的时候，却发现我们无法在他的原有的代码上把他的工作进行下去，原因的他的整个思路太混乱了，我们无法读懂它。
这样的类即使完成，在测试和维护的时候，也是困难重重的。一个不容易读懂的代码，是最难维护的。因为即使是类设计和编码人员来亲自维护，也是很难的，随着时间的推移，他们会渐渐的忘掉这些代码的功能和原来的设计思路，然后在他们维护的时候，就会感到相当的头疼，即使这些是他们自己写的代码。
将这些功能一个一个的分开吧！让它们在不同的实现类里，不管是分析、设计，还是编码，都是相当的容易的。
不要问我这个功能的相同的部分怎么重用，在设计模式里有好几种关于这样的问题的解决方案。最常用的就是模板方法。
分开以后，有关它们的单元测试就极为简单了，你可以一个一个功能的测，这些功能不会产生相互的影响。改
bug
你也有很强的针对性。同样，维护起来也是极有针对性的。
第四、
万精油式的工具类
这样的一些工具类也是什么都往里面塞，直到最后臭气熏天为止。最典型的是一些冠以
manager
名称的类。
这些类里面的功能都是相似的，但又相互之间没有重用价值，而且它们分跨不同的模块，经常会出现很多人抢着使用该类的情况，不管是在编码、测试和维护的阶段，总是有很多人在强着使用该类。
下面就是这样的一个例子：
public class DataManager
{
……
private static int getPort(List categorys,long str)
{
int retn = -1;
for(int i=0;i<categorys.size();i++)
{
DictCategoryValueModel dcvm = (DictCategoryValueModel)categorys.get(i);
if(dcvm.getCategoryValueId()==str) retn = i;
}
return retn;
}
public static ArrayList getSkill(SkillForm form)
{
ArrayList list = new ArrayList();
String[] selectedIds = form.getSelectedId();
for(int i=0;selectedIds!=null&&i<selectedIds.length;i++)
{
int j = Integer.parseInt(selectedIds[i]);
if(form.getType().equals("0"))
{
j--;
}
SkillModel model = new SkillModel();
model.setEmpId(form.getEmployeeId());
model.setType(form.getType());
model.setCateId(form.getCateId());
model.setSkillTypeId(form.getSkillTypeId()[j]);
model.setYearsOfExperince(form.getYearsOfExperience()[j]);
model.setVersion(form.getVersions()[j]);
model.setRecentYearofExperience(form.getYearsOfRecentExperience()[j]);
model.setDetails(form.getDetails()[j]);
list.add(model);
}
return list;
}
public static BasicInfoForm setBasicInfo(BasicInfoForm form,BasicInfoModel model)
{
form.setEmployeeId(String.valueOf(model.getEmployeeId()));
form.setCandidateId(String.valueOf(model.getCandidateId()));
form.setChineseName(model.getChineseName());
form.setEnglishProficiencyId(String.valueOf(model.getEnglishProficiencyId()));
form.setFirstName(model.getFirstName());
//form.setLastModifiedDate(Util.dateToString(model.getLastModifiedDate()));
form.setLastName(model.getLastName());
form.setLocationId(model.getLocationId());
form.setPositionId(model.getPositionId());
form.setSubmittedBy(model.getSubmittedBy());
form.setSubmittedDate(model.getSubmittedDate());
form.setDepartmentId(model.getDepartmentId());
EducationModel[] educations = model.getEducations();
form.setEmail(model.getEmail());
int len = educations.length;
int[] degreeIds = new int[len];
String[] majors = new String[len];
String[] graduatedSchools = new String[len];
String[] graduatedDates = new String[len];
for(int i=0;i<len;i++)
{
degreeIds[i] = educations[i].getDegreeId();
majors[i] = educations[i].getMajor();
System.out.println("***********"+majors[i]);
graduatedSchools[i] = educations[i].getGraduatedSchool();
graduatedDates[i] = Util.dateToString(educations[i].getGraduatedDate());
}
form.setDegreeIds(degreeIds);
form.setGraduatedDates(graduatedDates);
form.setGraduatedSchools(graduatedSchools);
form.setMajors(majors);
form.setRoleIds(model.getRoleIds());
//for log
form.setLastModifiedDate(model.getLastModifiedDate().toString());
return form;
}
……
private static void deleteOld(List list,long id)
{
for(int i=0;i<list.size();i++)
{
ClassifyExperienceModel model = (ClassifyExperienceModel)list.get(i);
if(model.getCategoryId()==id)
{
list.remove(i);
break;
}
}
}
……
}
作者试图把所有的
action
的一些数据之间的转化，即
actionForm
和后台的
POJO
之间的数据想换放在一个
DataManager
类中。这样的集中式管理并没有什么好处，因为各模块的数据交换各不相同。坏处却有，就是大家会抢着使用这个类。所以还是把这样的
Manager
类分拆到各个模块，让它们自己管理自己的数据吧。
一些工具类也容易写得过于庞大，比如，有人在
util
包里设计了一个
Helper
类，然后各模块有什么需要的功能都往这个
Helper
类里面加。随着编码阶段的结束，这个
Helper
类也变得相当的臭。
即便是工具类的功能，也不是随便找一个工具类就往里面放的。一定要让这个工具类只实现它自己的固有功能，其他的功能就不要随便往里面添加了。
总结
沙士比亚曾经说过，简洁是机智的灵魂。尽量把我们的代码写得简单易懂，这对谁都好。一个类写得过于复杂，这往往是我们没有分析和设计好的预兆。在我们设计一个类的时候，首先要牢牢记住的第一个面向对象的基本原则——单一职责原则：就一个类而言，应该仅有一个引起它的变化的原因。
扫一扫
1758
被折叠的 条评论
为什么被折叠?
到【灌水乐园】发言
