Should I extract specific functionality into a function and why?

Question

I have a large method which does 3 tasks, each of them can be extracted into a separate function. If I'll make an additional functions for each of that tasks, will it make my code better or worse and why?

Obviously, it'll make less lines of code in the main function, but there'll be additional function declarations, so my class will have additional methods, which I believe isn't good, because it'll make the class more complex.

Should I do that before I wrote all the code or should I leave it until everything is done and then extract functions?

Answer 1

This is a book I often link to, but here I go again: Robert C. Martin's Clean Code, chapter 3, "Functions".

Obviously, it'll make less lines of code in the main function, but there'll be additional function declarations, so my class will have additional methods, which I believe isn't good, because it'll make the class more complex.

Do you prefer reading a function with +150 lines, or a function calling 3 +50 line functions? I think I prefer the second option.

Yes, it will make your code better in the sense that it will be more "readable". Make functions that perform one and only one thing, they will be easier to maintain and to produce a test case for.

Also, a very important thing I learned with the aforementioned book: choose good and precise names for your functions. The more important the function is, the most precise the name should be. Don't worry about the length of the name, if it has to be called FunctionThatDoesThisOneParticularThingOnly, then name it that way.

Before performing your refactor, write one or more test cases. Make sure they work. Once you're done with your refactoring, you will be able to launch these test cases to ensure the new code works properly. You can write additional "smaller" tests to ensure your new functions perform well separably.

Finally, and this is not contrary to what I've just written, ask yourself if you really need to do this refactoring, check out the answers to "When to refactor ?" (also, search SO questions on "refactoring", there are more and answers are interesting to read)

Should I do that before I write all the code or should I leave it until everything is done and then extract functions?

If the code is already there and works and you are short on time for the next release, don't touch it. Otherwise, I think one should make small functions whenever possible and as such, refactor whenever some time is available while ensuring that everything works as before (test cases).

Answer 2

Yes, obviously. If it is easy to see and separate the different "tasks" of single function.

1. Readability - Functions with good names make it explicit what code does without need to read that code.
2. Reusability - It is easier to use function that does one thing in multiple places, than having function that does things you don't need.
3. Testability - It is easier to test function, that has one defined "function", that one that has many of them

But there might be problems with this:

• It is not easy to see how to separate the function. This might require refactoring of the inside of the function first, before you move on to separation.
• The function has huge internal state, that is passed around. This usually calls for some kind of OOP solution.
• It is hard to tell what function should be doing. Unit test it and refactor it until you know.

Answer 3

The problem you are posing is not a problem of coding, conventions or coding practice, rather, a problem of readability and ways text editors shows the code you write. This same problem is apearing also in the post:

Is it OK to split long functions and methods into smaller ones even though they won't be called by anything else?

Splitting a function into sub-functions make sense when implementing a big system with the intent to encapsulate the different functionalities it will be composed of. Nevetheless, sooner or later, you will find yourself with a number of big functions. Some of them are unreadeable and difficult to maintaint wether you keep them as single long functions or split them is smaller functions. This is particularly true for the functions where the operations you do, are not necessary in any other place of your system. Lets pickup one of such a long function and consider it in a broader view.

Pro:

• Once you read it, you have a complete idea on all the oprations the function does (you can read it as a book);
• If you want to debug it, you can execute it step by step without any jump to any other file/part of the file;
• You have the freedom to access/use any variable declared at any stage of the function;
• The algorithm the function implements its fully contained in the function (encapsulated);

Contra:

• It takes many pages of your screen;
• It takes long to read it;
• It is not easy to memorize all the different steps;

Now lets imagine to split the long function into several sub-functions and look at them with a broader prospective.

Pro:

• Except the leave-functions, each function describes with words (names of sub-functions) the different steps done;
• It takes very short time to read each single function/sub-function;
• It is clear what parameters and variables are affected at each sub-function (separation of concerns);

Contra:

• It is easy to imagine what a function like "sin()" does, but not as easy to imagine what our sub-functions do;
• The algorithm is now disapeared, it is now distributed in may sub-functions (no overview);
• When debugging it step by step, it is easy to forget the deepness level function call you are coming from (jumping here and there in your project files);
• You can easily loose context when reading the different sub-functions;

Both solutions have pro and contra. The actual best solution would be having editors which allow to expand, inline and for the full depth, each function call into its content. Which would make, splitting functions in sub functions the only best solution.

Answer 4

For me there are four reasons to extract code blocks into functions:

• You are reusing it: you just copied a block of code into the clipboard. Instead of just pasting it, put it into a function and replace the block with with a function call on both sides. So whenever you need to change that block of code, you only need to change that single function instead of changing the code multiple places. So whenever you copy a code block, you must make a function.

• It's a callback: It's an event handler or some kind of user code a library or a framework calls. (I can hardly imagine this without making functions.)

• You believe it will be reused, in the current project or maybe somewhere else: you just wrote a block that computes a longest common subsequence of two arrays. Even if your program calls this function only once, I would believe I will need this function eventually in other projects too.

• You want self-documenting code: So instead of writing a line of comment over a block of code summarizing what it does, you extract the whole thing into a function, and name it what you would write into a comment. Though I'm not a fan of this, because I like to write out the name of the algorithm used, the reason why I have chosen that algorithm, etc. Function names would be too long then...

Answer 5

I'm sure you've heard the advice that variables should be scoped as tightly as possible, and I hope you agree with it. Well, functions are containers of scope, and in smaller functions the scope of the local variables is smaller. It's much clearer how and when they are supposed to be used and it's harder to use them in the wrong order or before they are initialized.

Also, functions are containers of logical flow. There's only one way in, the ways out are clearly marked, and if the function is short enough, the internal flows should be obvious. This has the effect of reducing cyclomatic complexity which is a reliable way to reduce the rate of defects.

Answer 6

Aside: I wrote this in response to dallin's question (now closed) but I still feel it could be helpful to someone so here goes

---

I think that the reason for atomising functions is 2 fold, and as @jozefg mentions is dependent on the language used.

Separation of Concerns

The main reason to do this is to keep different pieces of code separate, so any block of code that doesn't directly contribute to the desired outcome/intent of the function is a separate concern and could be extracted.

Say you have a background task that also updates a progress bar, the progress bar update isn't directly related to the long running task so should be extracted, even if it's the only piece of code that uses the progress bar.

Say in JavaScript you have a function getMyData(), which 1) builds a soap message from parameters, 2) initializes a service reference, 3) calls the service with the soap message, 4) parses the result, 5) returns the result. Seems reasonable, I've written this exact function many times--but really that could be split into 3 private functions only including code for 3 & 5 (if that) as none of the other code is directly responsible for getting data from the service.

Improved Debugging Experience

If you have completely atomic functions, your stack trace becomes a task list, listing all the successfully executed code, i.e:

• Get My Data
• • Build Soap Message
• • Initialise Service Reference
• • Parsed Service Response - ERROR

would be allot more interesting then finding out that there was an error while getting data. But some tools are even more useful for debugging detailed call trees then that, take for example Microsofts Debugger Canvas.

I also understand your concerns that it can be difficult to follow code written this way because at the end of the day, you do need to pick an order of functions in a single file where as your call-tree would be allot more complex then that. But if functions are named well (intellisense allows me to use 3-4 camal case words in any function I please without slowing me down any) and structured with public interface at the top of the file, your code will read like pseudo-code which is by far the easiest way to get a high level understanding of a codebase.

FYI - this is one of those "do as I say not as I do" things, keeping code atomic is pointless unless your ruthlessly consistent with it IMHO, which I'm not.