Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question

migrated from stackoverflow.com Mar 7 '11 at 0:52

This question came from our site for professional and enthusiast programmers.

    
You've hit the nail on the head - as long as the code makes more sense in separate functions, do it. It will help you in the long run. –  Geoff Adams Mar 6 '11 at 23:58
1  
Does it matter what order authenticate or authorize are called in? If so then it can be valid to use a function to call both functions in the required sequence. –  Jacob Mar 7 '11 at 0:42
1  
Just ship the fkn thing. –  Job Mar 7 '11 at 3:11
    
i highly recommend you read the book 'Code Complete'. Second do look up on 'Cohesion and Coupling' in terms of software –  Imran Omar Bukhsh Mar 7 '11 at 17:08
add comment

6 Answers 6

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.

share|improve this answer
1  
Indeed. Computers understand even the weirdest crap. Humans don't. –  Jörg W Mittag Mar 7 '11 at 4:35
1  
Every function should tell a story. And a good story doesn't get into niggling details not germane to the main point of the story. Your character might tie his shoe, but you don't need to stop and tell the reader how he tied the shoe. That story is for a different level. –  Hack Saw Mar 7 '11 at 10:06
add comment

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!

share|improve this answer
    
thx for the replies guys. Yeah that's what I am constantly worried about, taking it to an extreme and perhaps breaking out a single method into too many methods, so i'm trying to figure out some sort of rule of thumb or general way to determine when it is the optimal situation to break up a method into smaller parts. –  blacktie24 Mar 7 '11 at 0:04
add comment

From the top suggested book for programmers on Stack Overflow - 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.

share|improve this answer
add comment

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.

share|improve this answer
add comment
  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)?
share|improve this answer
add comment

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...

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.