jeri_rl:

Yes, I'd suggest that you use subroutines in your subroutines. Generally the CPU impact is insignificant, but it can make maintenance so much simpler. In general, if you use good subroutine names, and restrict subroutines to doing a particular task, it will make your program *more* readable, rather than less.

I try to make a subroutine stick to a particular task, which tends to help keep them short. It also helps keep them reusable. Suppose I have a function called send_report and it loads up a mailing list from the database, formats an EMail, attaches a report and then sends it to everyone on the list. It's not terribly reusable. Sure, if you have to do something similar later, you can copy it and hack it up a bit. Or, you could chop it into smaller, more reusable parts, something like:

sub send_report {
   my ($list, $report, $template) = @_;

   my @recipients = get_list($list);

   my $email = format_EMail($template);
   
   send_EMail( to=>[@recipients], body=>$email, attachments=>[$report]
+ );
}
[download]

This function is pretty easy to read and understand. Sure, the code that does the bulk of the work is in a different subroutine, but that's a feature: If your EMail is formatted incorrectly, you know where to look: format_EMail. You don't have to worry about the code and variables that worries about sending EMail or getting a mailing list.

Another way to determine if your subroutine needs to be split up is to name a subroutine by what it's doing. If you're trying to name a subroutine do_this_and_that, the "_and_" is a tipoff that you need to split it up or that it'll be calling do_this and then do_that.

...roboticus

When your only tool is a hammer, all problems look like your thumb.

Should I make a new level of subroutines to have less repeating code? (In other words, my subroutines would have subroutines.) It seems that might be more confusing to decipher later.

If you name your (sub)subroutines well, to reflect what they do in the context of the callers, it can often greatly clarify things

Which way would be more efficient running the program?

You do pay a performance penalty for calling a subroutine. In any language, but usually more so in interpreted languages.

Or would it matter at all?

Whether the reduction in maintenance -- of not having to change (and remember to change) all the copies of similar code -- and the clarity and greater readability that can come from having smaller subs; thus less code to deal with at any given time -- is worth the performance cost -- is very much down to how critical performance is to your program.

If your program runs for a few minutes a couple of extra seconds don't matter much. If your program runs for hours and the extra subroutines add another hour or two; you might reconsider the balance.

If your program is IO-bound: eg waits for the user to fill in a web form; a few seconds might not matter too much; but a couple of minutes might loose you business.

Only you can know your application well enough to know the right answer to that question.

Should I make a new level of subroutines to have less repeating code? (In other words, my subroutines would have subroutines.)

The main reason for making subs is obviously to remove code duplication. If you find yourself copying and pasting a dozen or more lines of code, then those lines should probably go into a sub. Often even for less than a dozen lines.

Sometimes, subroutines are good even if they are called only once, because they hide the gory details from the main stream of thought. One of the litmus tests to figure out if it makes sense to make a sub is whether it is easy to give them a name that makes that sense. For example, I am often writing programs that need to compare data from different files. I might write something like this (untested code):

my $old_cust_data_href = load_old_cust_data($old_cust_data_file);
my $new_cust_data_href = load_new_cust_data($new_cust_data_file);
my $result_hash_ref = data_compare($old_cust_data_href, $new_cust_data
+_href);
# ...
[download]

my $old_cust_data_href = load_cust_data($old_cust_data_file);
my $new_cust_data_href = load_cust_data($new_cust_data_file);
my $result_hash_ref = data_compare($old_cust_data_href, $new_cust_data
+_href);
# ...
[download]

my $cust_data_href{$_} = load_cust_data($cust_data_file{$_}) for qw /o
+ld new/;
my $result_hash_ref = data_compare($cust_data_href{old}, $cust_data_hr
+ef{new});
[download]

Then, of course, there is the danger of getting too far. I remember having worked on a huge mass of C++ code where just about every single method had no more than two lines of code, so that you ended up with a messy spaghetti of method calls. Going to that extreme makes thing very difficult to maintain, what I sometimes call write-only code.

Update: modified the last code sample which was not fully consistent.

I just stick to what my Uni professors used to tell me about subs/functions - "Functions should do one thing, and do it well."

When it comes to clarity/readability/maintainablity, I stick to my 'C' roots and make everything possible a sub/function - that way when you have a list of calls to subs/functions, and any parameters passed to them, you can see clearly in what order things are working. IMHO anyway ;)

EDIT: And of course, appropriate naming conventions of subs/functions never hurt anybody! IMHO Try and avoid lengthy names, even as a relatively new professional programmer, all too often do I come across somebody elses subs like this this -> thisSubRoutineWorksOutThisNumberFromThatNumber - and I just think that becomes more of a hinderance then a help!

Thanks to everyone for all your comments. Seems like I had it backwards in terms of readability vs efficiency. I will take the advice of the majority and make more subroutines.

In my own view. Subroutines i create for my programs do one thing and do it well. Let the names of your subroutine tell you more about what lies within it and its ultimate function. e.g sub display_data().

Learn to reduce the length of you subroutines try to make them compact and straight to the point. things like first naming your variables before assigning values to them are way too long and repetitive. e.g my $name = undef; $name = qq{liray}
could be reduced to my $name = qq{liray};

Finally clarity, maintenance should be kept in mind, because you could visit this subroutine 6 months down the line and begin to wonder what you created it to do and why. Also learn to express your self in the best possible way that suits your style of programming.

“Efficiency,” most of the time, is not a major consideration.   “Clarity” always is.   If your subroutine breakdown as it stands right now is clear to you and more-or-less matches how you (and those who will follow you) “naturally think of” the task that the program is to solve, then I would not be overly concerned about repetition of source statements.   (Especially if the reasonable possibility exists that, someday, as the program is maintained over the coming years, the repeated statements might naturally diverge.)

While it is academically interesting to “spin-off common code into common subroutines,” this can lead to coupling of what are unrelated concerns.   If a particular piece of code is something that you think of as being truly common to several routines, and more-or-less “irrelevant” or “beside the point” to them, then spinning them off might improve clarity.

Another good idea would be to ask some of your co-workers to contribute a little bit of peer-review:   “Is this way of writing this logic clear to you?   Do you have any suggestions?”

Several people have said that a subroutine should "do one thing." In that spirit, there is a simple test for whether or not a block of code should be a subroutine. If the code can be completely described in one sentence with no 'and', 'or' or 'but', it probably should be a subroutine.

Note: This is not an original idea. It is paraphrased from Yordon and Constantine's 1979 book "Structured Design"

UPDATE: Corrected Reference.