Functions – Signs that your functions are doing more than one thing

We’ve established that the key to having small functions is to make sure that they do one thing. A guideline to follow is that functions should remain on a single abstraction level. Now we will mention two very easy to detect signs that our functions may be doing more than one thing.

Lots of indentation

streetfighterindentation

Why do we indent code? When we use a tab or spaces to “push” some lines of code, we do it because we want to make it visually evident that those lines form a block of code that does something specific. In some cases it is very valid to have some degree of indentation, but if you start noticing a lot of nested  if-else statements or loops that have other control structures within them then you should consider extracting some functions. We will now look at an example with a single level of indentation:

public void ExampleFunction(int x, int y){
  if (x > y)
  {
   // some lines of code that do something
   ...
   ...
   ...
   ...
   ...
  }
  else
  {
    // some lines of code that do something else
    ...
    ...
    ...
    ...
    ...
  }
}

Maybe the instructions inside the if-else statements aren’t very difficult to understand, maybe there’s not a lot of lines of code, but you could argue that this function is doing more than one thing. Maybe it would be better to extract the code inside the control structures and have the code look like this:

public void ExampleFunction(int x, int y){
  if (x > y)
    DoSomething();
  else
    DoSomethingElse();
}

This function is clearly a lot easier to read than the previous one, the specific instructions that do something or do something else have been extracted so we don’t have to look at them unless we need to.

Sections Inside Functions

This one is very easy to detect, because it manifests itself into large functions, that have several blocks of code where each one does a single task. Take for example a function that opens a text file, processes the data and then writes the data to a new file.

public void ProcessDataFile(string sourceFile, string destinationFile){

  // some code that opens the source file
  ...
  ...
  ...
  // some code that processes the data from the file
  ...
  ...
  ...
  // some code that writes the data to the new file
  ...
  ...
  ...
}

This function has several clearly defined sections where each one does a different thing. this function could be rewritten the following way:

public void ProcessDataFile(string sourceFile, string destinationFile){
  var data =  ReadDataFromSourceFile(sourceFile);
  ProcessData(data);
  WriteNewDataFile(destinationFile,data);
}

If you can see, a pattern emerges. We take a function and we start extracting new functions from it where the newly extracted functions have a name that clearly states what it does, and it does only one thing.

Use your common sense

Does this mean that every single function has to be only a few lines of code? Is it forbidden to have nested control structures? No, that’s not what it means, sometimes it will be valid for you to decide that you want to have a large functions for some reason. Sometimes it will be the right choice to have a few nested control structures. The key is to not go to one extreme or the other. You shouldn’t have functions that are thousands of lines of code, nor should you try to make all your functions only one or two lines of code. Our goal is to make sure that our functions do one thing, and that can mean different things in different situations.

Leave a Reply