dagblog.
← Archive

Open debate: small self-explanatory methods or large methods with comments?


During a code review I got absorbed in Martin Fowler's Refactoring, and I have to say I really like his programming style. I mean the way he writes methods and uses Extract Method. What I usually see, in my own work as well as others', is a longer method with comments, if any, that explain what each part does.

public Configuration getConfiguration() {



 // load the driver (supplied to installer) using custom classloader

 try {

 if (driverClass != null) {

 loadDriver(driverClass, dbDrivers);

 }

 } catch (ClassNotFoundException e) {

 throw new ConfigurationException(e);

 }



 // configure datasource

 BasicDataSource dataSource = new BasicDataSource();

 // Do not set the class name, becuase the datasource calls

 // Class.forName only in the systemclassloader where the driver

 // is not present. 

 // dataSource.setDriverClassName(driverClass);

 dataSource.setUrl(connectionURL);

 dataSource.setUsername(userName);

 dataSource.setPassword(password);

 dataSource.setInitialSize(1);

 dataSource.setMaxActive(15);

 dataSource.setDefaultAutoCommit(true);



 // create configuration instance

 Factory scopes = ScopeFactory.getInstance();

 Configuration cfg = new Configuration(scopes);

 cfg.setDataSource(dataSource);



 return cfg;

}

 
 

Java2html

Following Fowler's recipe, the refactored code might look like this.

public Configuration getConfiguration() {

 loadDriverInCustomClassloader(); 

 BasicDataSource dataSource = configureDataSource(); 

 return createConfigurationInstance(dataSource);

}



protected void loadDriverInCustomClassloader() { 

 try {

 if (driverClass != null) {

 loadDriver(driverClass, dbDrivers);

 }

 } catch (ClassNotFoundException e) {

 throw new ConfigurationException(e);

 }

}



protected BasicDataSource configureDataSource() {

 BasicDataSource dataSource = new BasicDataSource();

 // Do not set the class name, becuase the datasource calls

 // Class.forName only in the systemclassloader where the driver

 // is not present. 

 // dataSource.setDriverClassName(driverClass);

 dataSource.setUrl(connectionURL);

 dataSource.setUsername(userName);

 dataSource.setPassword(password);

 dataSource.setInitialSize(1);

 dataSource.setMaxActive(15);

 dataSource.setDefaultAutoCommit(true);

}



protected Configuration createConfigurationInstance(BasicDataSource dataSource) { 

 Factory scopes = ScopeFactory.getInstance();

 Configuration cfg = new Configuration(scopes);

 cfg.setDataSource(dataSource);

 return cfg;

}

 
 

Java2html

Fowler gives three main reasons for this approach.

  • Finer-grained methods are easier to reuse within the class.
  • They are easier to override in a subclass.
  • They are easier to read. Larger methods read like a series of comments.

To me the last point matters most: the readability of the code itself. It may not be obvious from my example, but when methods stretch out like spaghetti, this seems like an ideal way to make the code more readable and thereby improve its quality.

Medium version

Related articles

Related CZ Podcast episodes