bad smell code # 3 Large class

I took a look at our current code base; I found that the top 3 largest classes are as following:

CustomerDetailsForm1 (12261 LOC)

 

TransactionBO (LOC 9066)

 

DLPreRequisiteUserControl ( LOC 8124)

 

I have looked around on what is the right size of a class in term of LOC, there are many numbers popped out, but it is pretty much common understanding that one thousand LOC can be considered as too large, whither it is in Java or .Net program…

 

One of the articles I found stated the following:

<300 lines: fine
300-500 lines: reasonable
500-1000 lines: maybe ok, but plan on refactoring
>1000 lines: definitely refactor

With that I took a second look at our code base. Out of about 6000 classes,  

 

> 5000: 11

Between 5000 and 2000: 58

Between 2000 and 1000: 101

 

This shows how far away we are from having a high quality code base… maybe we can set a short term goal as make them all under 5K. Now; let’s look at how to break big classes into smaller ones.

 

When a class is trying to do too much, it is often shows up as too many members (methods) and/or too many instance variables, when a class has too many members or instance variables, duplicated code cannot be far behind.

 

There are 3 common approaches in making a class smaller.

<![if !supportLists]>1.       <![endif]>Make them smaller by eliminating duplicated code. Please refer to the first 2 articles addressing duplicated code and long methods.

<![if !supportLists]>2.       <![endif]>Push some logic that does not belong to it. If we let a class to do something that is not part of its responsibility, we could quickly put on weight on the class. The goal of this process is to push the logic to where it belongs

<![if !supportLists]>a.       <![endif]>For UI class, ask yourself should this logic to be implemented in UI, is Business layer or entity layer a better place for it?

<![if !supportLists]>b.      <![endif]>For Data Access class, ask yourself does it related to data access or is it some kind of business logic should be in Business layer or entity layer?

 

<![if !supportLists]>3.       <![endif]>Break it up into smaller class. If we try to let a class to do too many things, the class could be very big.  One of the OOP principles is SoC (Separation of Concerns). Following that principle, we should clearly define the boundary of each class. Take an example. Let’s say we have a business class call Customer. It is reasonable to assume this calls its respective Data Access class to access database when it need to retrieve or persist data, and it would not provide functionality which is not directly related to customer domain. When a class has grown too big, we can redefine the boundary of the class so that the large class can be broken into smaller ones by grouping its functionality into a few groups. Take an example, if we break our TransactionBO into multiple smaller classes, one for Title and Registration transactions, one for Driving License transaction, one for Plate Card…. etc.. Of course, we can put some common functionality into a base class so that they are available in all its subclasses. The LOC of these classes will be much smaller than what it is now in TransactionBO.

 

If your entity class is too big, the most common way is to extract a port of the entity into another entity class reference to it.

 

If your GUI class (form or user control) is too big, first thing you want to do is to investigate whether there are business logic implemented in UI layer which is better in business layer or entity layer. If that is not enough, you would want to investigate opportunities to break some potation of this UI into a user control by itself.

 

Another observation I have is during the development phase (only in development phase, NOT in stabilization phase), if a class is too hot, many developers are fighting to check it out for their work, or there are many change sets checked in on the same class, it is an indication that it might do ourselves good to break the class into small ones.