A discussion I had today at work with one co worker.
Is it worth creating a separate method, if it is going to be called just from a single location?. True, may be in the future you will have to call it from more places, but for the sake of the argument let's assume that it is known that it will never be called from any other place.
If yes: why? or when (under what conditions)?
If not: why not? And what would be the minimum number of callers a method must have?
Yes, if it helps readability and/or testability. Consider a method which does "quite a lot of work" (e.g. "get the specified web page") where the work can be broken up into individual steps, each of which only needs a few bits of information from the previous steps. Breaking up that work makes each step more testable and readable: you can get a high level view of what the method is doing by reading the steps in turn, and you can easily focus on any one particular step by reading the code for just that step.
Yes. Long methods are hard to read and maintain.
Logically related operations should be moved to one method. For example:
public void buildHouse() {
layFoundations();
buildWalls();
plaster();
buildRoof();
}
So, at least three important characteristics:
readability - that is far more readable than if all the code required for building the house was in this method body.
maintainability - imagine if you had 500 lines of mixed code for building the house, and you had to debug a problem in the roof.
testability - unit testing is also something to consider. You might want to test each operation separately.
Update: recently I was asked this question on an interview, and they introduced me to a downside I was surprised with - performance. In case you are going to have thousands of subsequent calls to your method, then having additional method calls inside it damages performance, even though putting a method call on the stack isn't that much expensive per-se. Of course this is a very rare case and you must be certain that performance will really suffer (by benchmarking it)
If code reuse is not an issue, then readability is. Creating separate method for each functionally complete code block makes is more understandable, if you give it a right name.
The answer, at least for me, is pretty much always yes. If you were writing a public method called "CSVFileToDataTable", it'd probably be quite a long method, which could be refactored into:
public DataTable CSVFileToDataTable(string pathToCSVFile)
{
string[] lines = GetLinesFromCSVFile(pathToCSVFile);
var headers = GetHeadersFromCSVFile(pathToCSVFile);
// uses the headers to create column names and the data in the "lines" to infer datatypes
var data = GetDataTableStructure(headers, lines);
PopulateDataTable(data, lines);
return data;
}
That example is a lot easier to read, and debug, IMO =)
In addition to the maintainability and readability factors, it greatly aids running a debugger when functionality is grouped. (This is more relevant for functions than methods.) If you can simply skip over a function, it is generally easier than trying to set a break after the inlined functionality.
Yes. If the method does just one thing, then it can easily be tested. Of course you can test any method, but as soon as the complexity increases it is more difficult to ensure test coverage of all decision paths.
Its internal state becomes encapsulated, therefore more predictable. Ever debugged a long method and found that another part of the same code was using "i" as a counter also?
By the same token, code maintenance/ bug fixes are also easier because you can more easily find the source of an error. Imagine a task that does three things and it takes about 25 lines of code. Ask yourself, would you rather debug 25 lines of code, or debug "first thing", "second thing" , "third thing"?
Give methods descriptive names and document their possible in/out behaviour. For example: This method returns the item or it returns null when the item is not found. Try not to let methods have unexpected side-effects, i.e. changing global variables.
Coding is about communicating your intent. If a method helps make your intent clear, then it is absolutely worth it.
A method can give a name to an operation that might otherwise be unclear. For example, let's say you wanted to check if a number is even:
if(n % 2 == 0)
Anyone who reads that line must understand the modulo operator, then reverse-engineer the trick, to finally realize what is being checked. This is much more intention-revealing:
if(IsEven(n))
It also tells the reader exactly why the modulo operator is being used:
private static bool IsEven(int n)
{
return n % 2 == 0;
}
Yes. In addition to previously stated reasons - you would probably want to unit-test your code (at least you should). Smaller methods makes this a lot easier. Splitting the code up in seperate methods will also function as a kind of commenting - as long as your method names are good.
Yes -- primarily for readability but consider your sanity when debugging later. If you have error reporting that logs the call stack, would you rather have to wade through the long function or jump to one of the smaller methods as specified by the call stack?
Also consider the cyclomatic complexity when you have to test the big methods.
Yes, I like to think of code in terms of Legos.
The more focused and discrete methods and functions are (smaller Legos) the more flexible they are when being rearranged without having to modify existing code. Big lumbering methods and functions (large Legos), not so much.
Any method you write that does something useful will eventually be used else where. If you write it well it will be easy to generisze and will become more useful with time. One large blob of code that you need an hour to figure out what it does will never be useful somewhere else.
Of course you can take this to the extream and then have thousands of unused underused or duplicated functions, but thats just a part of maintaining a large body of code.
If you read any of the suggested advice by Bob Martin (Uncle Bob) or various other articles on the topics of good design and clean code I think you will find there is consensus that YES, every method should be broken out even if it is only used once.
There are a couple of reasons:
Code readability, if you name the method something like ProcessUserGroup, that is much more readable than 8 lines of code and a comment at the top that says "//Process user group" sprinkled in...
Single Responsibility Principle, each method should be responsible for handling its own Action and any methods within that method should be responsible for handling their own actions, etc. etc. This creates a very clean and modular world in your code, and makes unit testing each method independently substantially easier.
Cyclomatic Complexity, typically breaking your code out into individual methods will reduce cyclomatic complexity.
In most real world scenarios, eventually that method will probably find a use again, more often than not when designing software a method I though would never ever, be used more than once, has been used more than once later down the road. This is obviously not always the case but it is another reason...
I can not think of a single reason NOT to break a method out, especially with todays Refactoring tools built into nearly all development IDE's it takes mere seconds to break a code out into its own method. It should ALWAYS be done, and when you have put several hours, days, months into that part of the application and are done and go back you will appreciate breaking those methods out, and so will the other developers on your project.