Should a long method only be used once in its class or function?
Many times in the code on the Internet or in the codes of my colleagues, I see that they create an object with only one method, which is used only once in the whole application. Like this:
class iOnlyHaveOneMethod{
public function theOneMethod(){
//loads and loads of code, say 100 of lines
// but it only gets used once in the whole application
}
}
if($foo){
$bar = new iOnlyHaveOneMEthod;
$bar->theOneMethod();
}
This is really better:
if($foo){
//loads and loads of code which only gets used here and nowhere else
}
?
It makes sense to move loads and loads of code for readability, but shouldn't that be a function?
function loadsAndLoadsOfCode(){
//Loads and loads of code
}
if($foo){ loadsAndLoadsOfCode(); }
Is it better to move the code to a new object and then just create a function or put the code right there?
To me the functional part makes more sense and seems more straightforward than creating an object, which is unlikely to be useful since it just holds one method.
source to share
The problem is not with it, but with the function or object.
The problem is that you have hundreds of lines in one blob. Whether this mass of code in an object method or just a class seems more or less irrelevant to me, just being minor syntactic sugar.
What are these hundreds of lines? This is the place to look for object-oriented best practices.
If your other developers really think that using an object instead of a function makes it much more "object oriented", but having a few hundred linear functions / methods is not considered a code smell, then I think you have some organizational education.
source to share
Well, if the method does have "loads and loads" of code, then it should be split into several protected methods in this class, in which case the use of class scope is justified.
Perhaps this code cannot be reused because it has not been well accounted for in several different ways. By moving it into a class and breaking it down, you may find that it is better used elsewhere. At least that would be much more convenient.
source to share
While a function with hundreds of lines of code clearly indicates the problem (as others have already pointed out), placing it in a separate instance class rather than a static function has advantages that you can take advantage of by re-coding your example fraction:
// let instead assume that $bar was set earlier using a setter
if($foo){
$bar = getMyBar();
$bar->theOneMethod();
}
This gives you a couple of benefits:
-
This is a simple example of a Strategy Pattern . if it
$bar
implements an interface that it providestheOneMethod()
, then you can dynamically switch with implementations of this method; -
Testing your class independently is
$bar->theOneMethod()
much easier, as you can replace with a$bar
mock while testing.
None of these benefits are available if you are just using a static function.
I would argue that while simple static functions have their place, non-trivial methods (as explicitly expressed in the "hundreds of lines" comments) deserve their own instance anyway:
- to separate concerns;
- to help testing;
- to help refactoring and re-implementation.
source to share
You are really asking two questions:
- Is it just declaring a function better than creating an object to hold just that function?
- If any function contains "code loads"?
First part. If you want to switch functions dynamically, you may need to explicitly encapsulate objects as a workaround in languages that cannot handle functions this way. Of course, you need to allocate a new object, assign it to a variable, and then call a function from that variable a bit dumb when all you want to do is call a function.
Second part: ideally not, but there is no clear definition of "loads", and in some cases it may be appropriate.
source to share
I would say that you almost never want to have a block or a method with a lot of code in it - it doesn't matter if it's its own class or not.
Moving it into an object can be the first step in refactoring, although it might make sense this way. Move it to your class first and then split it into several smaller methods.
source to share
Ok, I would say it depends on how closely the code block is related to the calling code section.
If it's so tightly coupled that I can't imagine being used elsewhere, I'd rather stick with it in a private method of the calling class. This way, it will not be visible to other parts of your system, ensuring that it will not be used by other people.
On the other hand, if the block of code is generic enough (email authentication, i.e.) might be of interest to other parts of the system, I won't have a problem extracting that part into a class of its own, and then consider that being a useful class ... Even if it means it will be a class with one method.
If your question was more on the lines of "what to do with hundreds and hundreds of lines of code", then you really need to do some refactoring.
source to share
I think I would like to rephrase the question you are asking. I think you want to ask questions, this is my singles responsibility class . Is there a way to decompose chunks of your class into separate small parts that can change independently of each other (data access and parsing, etc.). Can you unit test your class easily ...
If you can say yes to the above elements, then I wouldn't bother with the method and the new class, since the point here is that you have readable, maintainable code.
In my team, we have a red flag if the class is long (compared to the number of lines), but this is just a heuristic, as if you have a class of 2000 lines of codes, it can probably be broken and probably doesn't support SRP ...
source to share
For testability, it is definitely better to split it into a separate class with separate method (s). It is much easier to write unit tests for single methods than as part of an inline if statement in a code file or whatever.
Having said that, I agree with everyone that the method should be broken down into single responsibility methods instead of hundreds of lines of code. It will also make it more readable and easier to test. And hopefully you can get some reuse from some of the logic contained in this big mess of code.
source to share