When to refactor a method

Question

This is intended to be more of a discussion than an outright question, hence I've made it community wiki. When do you choose to break a method into smaller methods.

Say I have a method that has one purpose, i.e. to load a configuration file

public function configure($file){
    $info = path_info(file);
    $ext = $info['extension'];

    switch($ext){
    case 'ini':
    //code to parse ini files
    break;

    case 'xml':
    //code to parse xml files
    break;

    case 'php':
    //code to parse php files
    break;
}

}

personally, I'd break out to internal methods depending on the outcome of the switch - but say I have a more sequential method, a series of steps - i.e.

public function doSequence(){
    $stepOne = //some database access to retrieve values
    $stepTwo = //the result of some interpretation of values
    $stepThree = //a fresh database interaction based on the interpreted values
    $stepFour = //prep values for return
    return $values
}

So, what is your guiding principle for refactoring methods into the appropriate level of granularity?

Answer 1

You do not need to split a method, unless you want to reuse part of its code.

So, if you want to parse an ini-File in some other place, but don't need the whole configure function, then you break down the method.

In all other cases it doesn't make sense and should be left as is.

Answer 2

You refactor a method when you cannot in good conscience say that you immediately understand completely what it does. Usually, that means it cannot have more lines than you can see at one glance. Calling several subroutines that do complicated things is okay even if you don't know exactly how they do it - after all, that is what abstraction is all about. Good descriptive subroutine names are very helpful here

Answer 3

The basic situations when we refactor are:

1. Reuse: A method is broken down to methods m1, m2, m3, m4. Out of these m2 and m4 are reused.

2. Maintainability: The code in the method has grown up and will be hard to maintain.

3. Logical grouping: A method performs a number of tasks and they are logically separable. Refactor the method to have as many logical groups as the tasks represent.

This list can get longer.

Answer 4

As a rule of thumb: if you describe your method in plain english and the description contains an AND, then it's a good candidate for breaking the method down into smaller chunks. Basically, a method should be understandable in less than 30s. If not, refactor.

In your example above, the code to parse the config files should not be inside this method or even in the same class. An Ini File Parser has one responsibility: parsing ini files. A YAML file parser parses YAML files and so on. Make the actual parsers into separate classes. Give them an interface. Then aggregate the parser into the Config class.

Answer 5

Personally I refactor a method to make it as atomic as possible.

function loadFileAndSave()
{
   this.openFile("file name");
   this.removeLineBreaks();
   this.saveToDisk();
}

When coding this way you could almost get away with not doing low level documentation because the code reads well itself.

Answer 6

This question is a lot more complex then it may seem and involves looking at the entire design of your software, rather than just your examples. There are good books on software design, and they cannot be summarized in a paragraph.

For example, it might not be a good idea to do the database accesses in the same object that is part of the model. Database access may be seen as more of a technical service, unrelated to the logic of your program, hence may need a separate class (and this aids reusability). But it all depends.