Should a long method used only once be in its own class or in a function?

Question

A lot of times in code on the internet or code from my co-workers I see them creating an Object with just one method which only gets used once in the whole application. Like this:

 class iOnlyHaveOneMethod{
   public function theOneMethod(){
     //loads and loads of code, say 100's of lines
     // but it only gets used once in the whole application
   }
 }

 if($foo){ 
  $bar = new iOnlyHaveOneMEthod;
  $bar->theOneMethod();
 }

Is that really better then:

if($foo){
 //loads and loads of code which only gets used here and nowhere else
}

?
For readability it makes sense to move the loads and loads of code away, but shouldn't it just be in a function?

function loadsAndLoadsOfCode(){
 //Loads and loads of code
}
if($foo){ loadsAndLoadsOfCode(); }

Is moving the code to a new object really better then just creating a function or putting the code in there directly?
To me the function part makes more sense and seems more readible then creating an object which hardly is of any use since it just holds one method.

Answer 1

Well, if there really is "loads and loads" of code in the method, then it should be broken down into several protected methods in that class, in which case the use of a class scope is justified.

Perhaps that code isn't reusable because it hasn't been factored well into several distinct methods. By moving it into a class and breaking it down, you might find it could be better reused elsewhere. At least it would be much more maintainable.

Answer 2

The problem is not whether it's in a function or an object.

The problem is that you have hundreds of lines in one blob. Whether that mass of code is in a method of an object or just a class seems more or less irrelevant to me, just being minor syntatic sugar.

What are those hundreds of lines doing? That's the place to look to implement object oriented best practice.

If your other developers really think using an object instead of a function makes it significantly more "object oriented" but having a several-hundred line function/method isn't seen as a code smell, then I think organisationally you have some education to do.

Answer 3

yes, the presences of loads and loads of code is a Code Smell.

Answer 4

I'd say you almost never want to have either a block or a method with loads of code in it -- doesn't matter if it's in it's own class or not.

Moving it to an object might be a first step in refactoring 'though - so it might make sense in that way. First move it to its own class and later split it down to several smaller methods.

Answer 5

Well, I'd say it depends on how tightly coupled the block of code is with the calling section of code.

If it's so tightly coupled, that I can't imagine it being used anywhere else, I'd prefer sticking it in a private method of the calling class. That way it won't be visible to other parts of your system, guaranteeing it won't be misused by others.

On the other hand, if the block of code is generic enough (email validation i.e.) to possibly be interesting in other parts of the system, I'd have no problem extracting that part into it's own class, and then consider that to be a utility class. Even if it means it will be a single-method class.

If your question was more in the lines of "what to do with hundreds and hundreds of lines of code", then you really need to be doing some refactoring.

Answer 6

As much as a single method with lots of code is a code smell. My first thought was to at least make the method static. No data in the class so no need for creating an object.

Answer 7

I think i would look to rephrase the question that you are asking. I think you want to ask the questions is my class supporting singles responsibility principle. Is there anyway to decompose the pieces of your class into seperate smaller pieces that might 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 items, i wouldn't worry about method versus new class as the whole point here is that you have readable, maintainable code.

In my team we have red flag if a class gets long (over x amount of lines) but that is just a heuristic as if you class has 2000 lines of codes it probably can get broken down and is probably not supporting SRP.

Answer 8

Whilst the function with hundreds of lines of code clearly indicates a problem (as others have already pointed out), placing it in a separate instance class rather than a static function does have advantages, which you can exploit by rejigging your example a fraction:

// let's instead assume that $bar was set earlier using a setter
if($foo){ 
    $bar = getMyBar();
    $bar->theOneMethod();
}

This gives you a couple of advantages now:

• This is a simple example of the Strategy Pattern. if $bar implements an interface that provides theOneMethod() then you can dynamically switch implementations of that method;

• Testing your class independently of $bar->theOneMethod() is dramatically easier, as you can replace $bar with a mock at testing time.

Neither of these advantages are available if you just use a static function.

I would argue that, whilst simple static functions have their place, non-trivial methods (as this clearly is by the 'hundreds of lines' comment) deserve their own instance anyway:

• to separate concerns;
• to aid testing;
• to aid refactoring and reimplementation.

Answer 9

You are really asking two questions here:

• Is just declaring a function better than creating an object to hold only this function?
• Should any function contain "loads of code"?

The first part: If you want to be able to dynamically switch functions, you may need the explicit object encapsulation as a workaround in languages that cannot handle functions this way. Of course, having to allocate a new object, assign it to a variable, then call the function from that variable is a bit dumb when all you want to do is call a function.

The second part: Ideally not, but there is no clear definition of "loads", and it may be the appropriate thing to do in certain cases.

Answer 10

For testability, it is definitely better to break it out into a separate class with separate method(s). It is a whole lot easier to write unit tests for single methods than as part of an inline if statement in a code-behind file or whatnot.

That being said, I agree with everyone else that the method should be broken out into single responsibility methods instead of hundreds of lines of code. This too will make it more readable and easier to test. And hopefully, you might get some reuse out of some of the logic contained in that big mess of code.