|
lyapunov has asked for the wisdom of the Perl Monks concerning the following question:
Dear PerlMonks, one of the things that I have gotten religion about, particularly after reading "Effective Perl Programming" by Hall, McAdams, and foy, is using strict.
Recently, I am trying to build a few generalized subroutines for tasks that I have done over and over, with the eventual goal of building my own module. The goal of making these routines as reusable as possible, has led me to, at least hopefully, make better design desisions.
As a great deal of my work involves system administration. I created the following routine to provide a generic interface for returning a date string from localtime for use in archiving logs files and the like. The idea is to call the subroutine with two arguments, the first being the format of the information that you want returned e.g. YYYYMMDD and the second argument is the offset from current time. Negative numbers take you back in time, positive numbers will generate a return from a future date.
As I have stated, I am writing all of my code to be strict compliant, but one of my peer's reviewed my code and suggested that making the second argument of the time_offset to be optional, so in the case that it is omitted, zero offset is assumed. I have done a fair amount of research, but have gone nowhere quickly.
I currently have the prototype as return_time($ $) but if I call it without the second argument, it bails. Is what I am trying to do possible, worthwhile> Would any of you mind leading me to an enlightned path?
FYI I try to do a reasonable job of documenting my code, but if you have any questions or concerns that are not addressed, please let me know. Also, having been on the receiving end of working on other peoples code, my overriding philosophy that I have adopted while writing code is taken from Conway's "Perl Best Practices" - "Always code as if the the guy who ends up maintaining your code will be a violent psychopath who knows where you live." It may not be as idiomatic and compact as possible, but I hope that it is at least not amateurish.
As always, any other design or critiques are welcomed. I am here to learn after all. Again, many thanks for your help and insight.
###############################
###############################
#
# subroutine_name: return_time
#
# Purpose: To provide a generic interface for extracting human readabl
+e
# values in an arbritrary order from epoch time.
#
#
#
# Arguments: This subroutine is designed to take two arguments:
#
# First required The format of the time string you want returned.
# Second optional The signed offset from the current time in second
+s.
# To go back in time use a negative number.
# To go forward use a positive number.
#
#
#
# Limitations: none
#
#
#
# Dependencies: perl 5.010
# pad_with_zeroes
#
#
#
# Description: After having to deal with stripping specific time
# formats out of localtime on a program-by-program basis this is an
# attempt to remedy the situation.
#
# To use this subroutine call it with a string where the values you wa
+nt
# out are in the order you want them out each specific piece of
# information needs to be separated by a colon.
#
# Here is the reference manual;
# YYYY the four digit year.
# YY the last two digits of the year.
# DD is the double digit day 01-31
# MMM 3 letter alpha name for the month in ALL CAPS
# Mmm 3 letter alpha name for the month with just the 1st letter
+ CAPPED
# MM The number of the month 01 for Jan - 12 for Dec
# julian the julian day of the year.
# hh hours
# mm minutes
# ss seconds
# Ddd the 3 alpha name of the weekday 1st letter CAPPED
# DDD the 3 alpha name of the weekday ALL CAPS
#
# Recall that there are 86400 seconds in a day.
#
# To get the the YYYYMMDD for yesterday call
# return_time("YYYY:MM:DD", -86400);
#
# To get the YYYY:julian for next week call
# return_time("YYYY:julian", 7*86400);
#
#
#
# Changelog:
# Date Engineer DR Change Description
# 28 Jul 11 Mike K Program Creation
#
#
###############################
###############################
sub return_time ($ $) {
#
# setup requirements, eat your greens
#
use Time::Local;
use 5.010;
#
# Data Structures needed for quick conversion of
# numeric values return by localtime into useable
# formats;
#
my %month_to_num = ("0","01",
"1","02",
"2","03",
"3","04",
"4","05",
"5","06",
"6","07",
"7","08",
"8","09",
"9","10",
"10","11",
"11","12");
my %month_to_name = ("0","Jan",
"1","Feb",
"2","Mar",
"3","Apr",
"4","May",
"5","Jun",
"6","Jul",
"7","Aug",
"8","Sep",
"9","Oct",
"10","Nov",
"11","Dec");
my %day_to_alphaday = ("0", "Sun",
"1","Mon",
"2","Tue",
"3","Wed",
"4","Thu",
"5","Fri",
"6","Sat");
#
# Variable declarations
#
my $input_string;
my @time_outputs;
my $offset_from_current_time;
my $output_time_string;
my $time;
my $temp;
my $cur_day_sec;
my $cur_day_min;
my $cur_day_hour;
my $cur_day_mday;
my $cur_day_mon;
my $cur_day_year;
my $cur_day_wday;
my $cur_day_yday;
my $cur_day_isdst;
#
# Grab the input string and split it into an array.
#
$input_string=shift;
$offset_from_current_time=shift;
#
# The code that I want to handle the case of the offset being zero
# in case the second argument is not included will go here.
#
# Basically if defined use that, else $offset_from_current_time=0;
#
#
@time_outputs=split ":", $input_string;
#
# Extract values into assorted variable from localtime
#
($cur_day_sec, $cur_day_min, $cur_day_hour, $cur_day_mday, $cur_day
+_mon,
$cur_day_year, $cur_day_wday, $cur_day_yday,
$cur_day_isdst)=localtime(time()+$offset_from_current_time);
#
# The return strings will be formed in the order given
# The following basically works like a case statement in
# shell.
#
# the variable temp is used so as not to modify the original data.
#
foreach $time (@time_outputs) {
given ($time) {
when ('YYYY') {
$temp=$cur_day_year+1900;
$output_time_string=$output_time_string."$temp";
}
when ('YY') {
#
# Barewords in strict are not allowed hence
# the qw in front of the unpack arguments.
#
$temp=$cur_day_year+1900;
(my $d1, my $d2, my $d3, my $d4)=unpack qw(A1A1A1A1),$temp
+;
$temp=$d3.$d4;
$output_time_string=$output_time_string."$temp";
}
when ('DD') {
$output_time_string=$output_time_string."$cur_day_mday";
}
when ('julian') {
$temp=pad_with_zeroes($cur_day_yday+1,3);
$output_time_string=$output_time_string."$temp";
}
when ('MMM') {
$temp=$month_to_name{"$cur_day_mon"};
$temp=~tr/a-z/A-Z/;
$output_time_string=$output_time_string."$temp";
}
when ('Mmm') {
$temp=$month_to_name{"$cur_day_mon"};
$output_time_string=$output_time_string."$temp";
}
when ('MM') {
$temp=$month_to_num{"$cur_day_mon"};
$output_time_string=$output_time_string."$temp";
}
when ('Ddd') {
$temp=$day_to_alphaday{"$cur_day_wday"};
$output_time_string=$output_time_string."$temp";
}
when ('DDD') {
$temp=$day_to_alphaday{"$cur_day_wday"};
$temp=~tr/a-z/A-Z/;
$output_time_string=$output_time_string."$temp";
}
#
# For hours minutes and seconds localtime does not return
# zero padded values. These are all padded to two digit
# lengths to remove ambiguity e.g. would 1411 mean
# 140101 or 141100 and so on.
#
when ('hh') {
$temp=pad_with_zeroes($cur_day_hour,2);
$output_time_string=$output_time_string."$temp";
}
when ('mm') {
$temp=pad_with_zeroes($cur_day_min,2);
$output_time_string=$output_time_string."$temp";
}
when ('ss') {
$temp=pad_with_zeroes($cur_day_sec,2);
$output_time_string=$output_time_string."$temp";
}
}
}
#
# Return the string
#
return $output_time_string;
}
################################
################################
#
# subroutine_name: pad_with_zeroes
#
# Purpose: This routine takes a single number and pads and arbitrary
+amount of zeroes in front of it.
#
# Limitations: none
#
# Dependencies:
#
# Description: This subroutine requires two arguments to be passed to
+it. The first is the
# the value you want padded with prepending zeroes. The second is the
+the length that you
# want the final string to be. If the value you want padded is greater
+ than the pad length
# nothing will change, e.g.
#
# pad_with_zeroes(1,2) will return 01
# pad_with_zeroes(1,3) will return 001
# pad_with_zeroes(1000,1) will return 1000
#
#
# Changelog:
# Date Engineer DR Change Description
# 06 Jul 11 Mike Kmetz N/A Program Creation
#
#
###############################
###############################
sub pad_with_zeroes ($ $){
#
# Variable declarations
#
my ($value, $pad_length);
#
# Load the values and do the work
#
$value=shift;
$pad_length=shift;
#
# We test to see if the length of the value is shorter
# than the length of the zero padding desired
# If it is the zeroes are prepended. If not the
# orginal value is returned.
if (length($value)<$pad_length ) {
do {
$value="0".$value;
}
until (length($value) eq $pad_length);
}
#
# Return the value.
#
return $value;
}
|
Re: How do I prototype a function with a varying number of arguments?
by AnomalousMonk (Archbishop) on Jul 29, 2011 at 18:00 UTC
|
Why do you want to use prototypes? They almost never do just what you want and/or expect. In any event, optional parameter values must be handled in the same way: by checking in the function that a defined (or otherwise valid) value has been passed.
>perl -wMstrict -le
"sub opt_w_proto ($;$) {
my ($x, $y) = @_;
$y //= 42;
print qq{x $x y $y};
}
;;
sub opt {
my ($x, $y) = @_;
$y //= 42;
print qq{x $x y $y};
}
;;
opt_w_proto(7);
opt_w_proto(7,0);
;;
opt(7);
opt(7,0);
"
x 7 y 42
x 7 y 0
x 7 y 42
x 7 y 0
| [reply]
[d/l] |
Re: How do I prototype a function with a varying number of arguments?
by koolgirl (Hermit) on Jul 29, 2011 at 18:55 UTC
|
I myself am doing the same exact thing as you, basically making my subroutines "generic", (at least any subroutine I write that could ever be used again) more for my own code library and the benefit of the practice than a module or anything, but I haven't really taken the approach of using prototypes. At the mention of making my subroutines generic, my guru suggested using them, but upon my own research, I just didn't see the benefit, seems a bit redundant to me, perhaps I'm missing their functionality/purpose?
Also, just food for thought, in doing this I've found it much easier to write a specific one time sub, then go back and change a few var names/etc to make it generic, than to write it generic the very first time through. I always feel a bit guilty about this, because I instinctively sense the waste of time and redundancy of writing something just to re-write it, but once you've got a huge library of very generic subs, it pays for itself many times over, and saves much more time than it wastes, IMHO anyway.
Oh, and JavaFan, very nice, I've never seen that in a sub prototype, ";" meaning an optional second scalar, I was about to suggest to lyapunov to just use an @ as the prototype, as your second example did, when I saw your reply and learned a new solution. ++
| [reply] |
|
|
| [reply]
[d/l]
[select] |
|
|
Your words are almost exactly what went through my head when I first began the research into prototypes, behind the suggestion I mentioned.
I'd say that prototypes make functions less generic.
I completely agree, I mean how is it, that putting a limitation on the number and type of arguments that can be passed to a function, make it generic and more suitable to use as a standby function?...That concept never made much sense to me. Glad someone else saw that. Now, as far as the reasons one might choose to use prototypes, I'm sure there are many great arguments, but for the point of creating generic functions, I agree that there doesn't seem to be a correlation.
| [reply] |
|
|
|
|
| |
|
| |
|
| |
|
Thank you all for the responses!
JavaFan, thanks for the quick insight!
AnomalousMonk, I have considered the question of using them, especially after reading Tom Christiansen's article but the reason that I use try to use them is that it sometimes helps me, in the same way using strict vars does. If I forget to feed a function call the needed requirements it lets me know. Truth be told, my learnedness and ability in Perl are below the threshhold where I think I will start seeing some of the bad juju.
koolgirl, the reason that I use prototypes is two fold, with strict it helps me debug crappy little things like tranpsosition errors, and it has been forcing me to think harder about what I want to feed it. The article linked above is really well written, but as I said, for me the juice is worth the squeeze as it helps with accounting problems on my end.
The other big push for me to build this library is that I am currently having to write a fair amount of code that does metrics from several different sources that need to be compiled together or have other special needs. I was beginning to see common refrains in the needed work. I do the write it once and revise, but I am enjoying the aspect of trying to write it in the most useful way possible first and see if I can anticipate unseen needs or problems and guard against them. Also, having seen a myriad of ways that people hobble together the same information, trying to anticipate those has been an even bigger pay off. In ways, I think this has helped me and my coding style quite a bit. But, I am boring geek, so I guess it goes with my territory.
| [reply] |
Re: How do I prototype a function with a varying number of arguments?
by eyepopslikeamosquito (Archbishop) on Jul 30, 2011 at 00:55 UTC
|
Your commenting style hit me in the face like a 1960s COBOL manual.
Generally, you are "over commenting".
You need to get to the point. Be precise and succinct.
Maintenance programmers are often
under time pressure. Don't force them to wade through
unnecessary comments,
"conversational" comments, or witty jokes to get to the meat.
A couple of examples of poor commenting from your code:
#
# setup requirements, eat your greens
#
This comment is unnecessary and jokes like these get old very quickly.
#
# Return the value.
#
return $value;
Don't belabour the obvious!
Perl Best Practices has an excellent chapter on Documentation, which I recommend you read.
Some commenting tips (taken from On Coding Standards and Code Reviews):
- Separate user versus maintainer documentation.
- Prefer to make the code obvious.
- Don't belabour the obvious.
A couple of other things I noticed:
- In addition to "use strict", don't forget about "use warnings".
- Don't put "use" statements inside subs (see Code style advice: where to put "use" statements?).
- Unpack subroutine arguments at the top of the sub. Update: This is discussed in Chapter 9, "Subroutines" of Perl Best Practices in the third item "Always unpack @_ first". Luckily, Chapter 9 happens to be the free sample chapter from this book.
- Declare variables at point of first use. Minimize variable scope. For example, remove the big ugly block of variable declarations near the top of your sub and replace "foreach $time ..." with "foreach my $time...".
- To make code easier to read, suggest white space around operators (e.g. $value="0".$value -> $value = "0" . $value).
- Don't use prototypes.
| [reply]
[d/l]
[select] |
|
|
The "unpack subroutine arguments at the top of the sub" and "declare variables at point of first use" don't always see eye to eye to each other. In fact, the former tends to create a "big ugly block of variable declarations near the top of your sub", something the latter doesn't want to see.
| [reply] |
|
|
In general, the consensus is that Perl 5's simple subroutine syntax is just a little too simple.
Well, okay, it's a lot too simple. While it's extremely orthogonal to always pass all arguments
as a single variadic array, that mechanism does not always map well onto the problem space.
-- Larry Wall in Apocalypse 6 (2003)
Yes, I see your point, so let me clarify.
Minimizing variable scope is a sound general programming practice
that I strive to employ in all languages I currently use (C++, Perl, Python, Ruby, Java).
That is my strategic goal.
This "unpack subroutine arguments at the top of the sub" business is
just a pragmatic tactic used to work around Perl 5's primitive subroutine syntax.
That is, in most languages,
the scope of function arguments is the whole function
and that is made plain by the language syntax.
Explicitly unpacking Perl's appallingly non-descriptive
$_[0], $_[1], ... subroutine argument names at the top of
the subroutine to more meaningful lexically scoped names
is usually the clearest way to express that these are indeed
the function's arguments and that their scope is accordingly
the whole function.
In the common case of read-only subroutine arguments, it is
also safer in that it avoids accidentally changing the
passed arguments.
The reasons behind this rule are described in detail
in Perl Best Practices, along with exceptions to this rule (e.g. short and simple subs,
when efficiency is critical, ...).
| [reply]
[d/l]
[select] |
|
|
And see On comments for a long discussion between BrowserUK and I about the meaning and value of comments. BrowserUK is strictly a minimalist (or perhaps a zeroist), and feels comments only get in the way, and I'm a semi-maximalist, who feels that comments should be there to clarify intent, point out pitfalls, and explain complex code to the reader. That said, I too agree that comments which simply mirror the code are not needed. In the example noted, the comment is not needed; if there were some specific limitation on what the return value should be or a special case that might occur, a comment would be a good idea. That's a marginal case and might or might not be better put in the POD for the subroutine (which should also be there); it would depend on the context. In an internal-use-only subroutine that was not part of a documented external interface, I'd use a comment; in a published, intended-for-use-without-reading-the-code situation, I'd put it in the POD.
| [reply] |
|
|
| [reply] |
|
|
|
|
Thank you all for the constructive criticism and food for thought. I have never enjoyed having a mentor. It has largely been on my own (I guess that shows). I have had the misfortune of dealing with other people's code where nothing was commented and the code was, at least for me, not intuitive in any way. As a matter of fact, I can't think of a single program that I have had to tweak that had comments. As a result, I have been over-compensating, I will take a page from your book and Strunk and White's, and keep it brief. I will rework this week and repost it to you, if you don't mind, to see if it is up to scratch.
Again, thanks. I am happy to have this place, and people like you that will help folks along the way. Cheers!
| [reply] |
Re: How do I prototype a function with a varying number of arguments?
by JavaFan (Canon) on Jul 29, 2011 at 17:56 UTC
|
sub return_time ($;$) # Optional second, scalar, argument
sub return_time ($@) # Optional arguments (list) after first, scal
+ar argument
| [reply]
[d/l] |
Re: How do I prototype a function with a varying number of arguments?
by dwm042 (Priest) on Jul 29, 2011 at 19:31 UTC
|
Not complaining about any of the code written here, but date & time solutions are vastly over engineered in the Perl module space, much as configuration file reading/writing are. You might want to look at DateTime, Date::Manip, Date::Calc, Date::Simple, etc. For comments and usage functions, please look at POD and Pod::Usage.
I can't begin to tell you how much time and effort I would have saved by taking a closer look at Config::Simple or perhaps YAML than brewing my own config parser.
D- | [reply] |
Re: How do I prototype a function with a varying number of arguments?
by ricDeez (Scribe) on Jul 29, 2011 at 23:30 UTC
|
- Some of your code could be made a bit clearer, your commenting style is a
bit distracting and you could benefit from using POD
- Some general comments which reflect my personal style but which you
may care to consider if it suits:
# use map where possible to create hashes where there
# is a clear relationship between the key and value pairs
my %month_to_num = # hash from map
map { my $prefix = ($_ < 9) ? "0" : ""; $_ => $prefix . ($_ + 1) }
+ ( 0 .. 11 );
# use fat comma to make code more readable
my %month_to_name = (
0 => 'Jan',
1 => 'Feb',
2 => 'Mar',
3 => 'Apr',
4 => 'May',
5 => 'Jun',
6 => 'Jul',
7 => 'Aug',
8 => 'Sep',
9 => 'Oct',
10 => 'Nov',
11 => 'Dec'
);
# this subroutine could be made much simpler
sub pad_with_zeroes {
my ($value, $pad_length) = @_;
my $pad = "0" x ($pad_length - length($value));
$pad . $value;
}
# avoid meaningless comments
# --------------------------
# e.g.
#
# Variable declarations
#
# and later
# ---------
#
# Load the values and do the work
# in my opinion this creates noise and does not aid readability
# Refer to Chapter 7 of Perl Best Practices and use POD
| [reply]
[d/l] |
Re: How do I prototype a function with a varying number of arguments?
by Anonymous Monk on Jul 31, 2011 at 06:41 UTC
|
Prototypes are an attempt to bolt type safety onto a typeless language. It doesn't usually end well except in a few very specific cases. Personally, I've never seen the point to using them. Just say no!
| [reply] |
|
|
| [reply] |
|
|
