How do you know when to split an object method into 2 or more other methods?

Question

I know this is a very basic question, but I sometimes find myself struggling to figure out when to split a single object method into multiple methods. For example, I am trying to set up an ACL using Zend_Acl and Zend_Auth, as shown in this tutorial: http://devzone.zend.com/article/1665. However, I am wondering if the My_Plugin_Auth::preDispatch() method should invoke calls to a method called authenticate() and a method called authorize(), instead of having everything lumped in under preDispatch(). I was thinking that this would make the code more readable and encapsulate the logic into its smaller parts, but i'm not sure if this is reason enough.

Answer 1

Without having read the article in question, certainly you should avoid multiplexing functionality in methods - authentication and authorization are very different things.

So on that basis alone, there's good reason.

his would make the code more readable ...not sure if this is reason enough.

As I've been spending years telling people, we do not write code for computers to understand - we write code for people to understand. So if splitting it into more methods make it easier to unerstand, then its a good idea.

Answer 2

From the top suggested book for programmers on Stack Overflow - https://stackoverflow.com/questions/1711/what-is-the-single-most-influential-book-every-programmer-should-read

Valid reasons to create a routine

Here are some that may be applicable to this case.

• Reduce complexity
• Avoid duplicate code
• Support subclassing
• Hide sequences - do the functions need to be called in a particular order?
• To ensure all routines are small? NO

Another point mentioned elsewhere is that a function should "have strong, functional cohesion - doing one and only one thing and doing it well".

If the sequence matter it may be worth creating a wrapper function that calls the two in the correct order. In an object oriented approach, create two separate functions for both the operations, make them private/protected and have a public method call the two in sequence. That way anyone using your class won't make the mistake of calling only one of the function or calling them in the wrong order.

Answer 3

I think it's a good idea to break up larger functions into logical components and from the names of your two methods it sounds very reasonable. Down the road you may need to be able to use that authorize method for something else without the call to authenticate and vice versa. That being said taking it to an extreme can be a bad thing too so be sensible!

Answer 4

This is a concept that I learned in Structured Programming and still applies to Object Oriented Programming, as well. I have seen a lot of "Spaghetti Object Oriented Code", with methods that have a lots of lines...

Here are quick ideas of when to cut a single function/method into several:

• Empirical 25 lines rule

If the code has more than 25 lines, because the programmer's mind, have difficult to understand more than that information.

• Loop rule

When you a "loop", with an specific goal, and that code is very extended. By "loop", I mean "for", "foreach", "while", etc. A "for" with 3 lines of code may not require to be put in a separate method, but a "for" with 35 lines of code, means move it.

Source Example (C#):

before_blah();
for (int k=0; k < list.Count; k++) {
  for_blah_1(k)
  ...
  for_blah_42(k)
}
after_blah();

Dest Example (C#):

before_blah();
this.iterateitems(); // <-- for goes there
after_blah();

• Conditional or Alternative rule

Similar, goes for conditional / alternative "blocks" like "if", "swith", "case", if some of the options is too extended, I make a single method for that option. There are times, that I even make a single method for each option, even if it not that large.

Source Example (C#):

switch (option) {
  case 0: 
    blah_0_0();
  break;
  case 1: 
    blah_1_0();
    ...
    blah_53_0();
  break;
} // switch

Dest Example (C#):

switch (option) {
  case 0: 
    cancelItem();
  break;
  case 1: 
    printItem();
  break;
} // switch

• Functional Rule

By "functional", doesn't mean "function", but activity. If you have 3 or more consecutives lines of code, that share a common purpouse, you may want to make a separate method.

Source Example (C#):

FileStream F = new FileStream();
F.Path = parameterPath;
F.Options = myOptions;
F.Reset();

do_something();

F.Close();

Dest Example (C#):

FileStream F = this.PrepareFileStream(parameterPath, myOptions);

do_something();

F.Close();

Of course, some of this guidelines are more subjective than logical.

Just my 2 cents...

Answer 5

Needing to reuse is an obvious reason. Readability is always beneficial, but I'm starting to use the theory of How much can I hold in my brain at one time? Break off a chunk of the code into another method and once you get it to work, you can forget about it. It helps me to focus.

Answer 6

1. Is it so long that I can't view it without having to page through it?
2. Does it do more than one thing at a time?
3. Is it difficult to tell what it's doing?
4. Does it show signs of Cut-Copy-Paste Reuse?
5. Do I have a perfectly valid reason to refactor the method at this time, or should I be doing something else (like fixing a defect)?