| Perl Doctor | So! What brings you in this morning, Mr. —Monk? |
Just Another Perl
Hypochondriac | Well, Doctor, you see, I have this module... |
| Perl Dr. | A swollen module, perhaps? |
| JAPH | Eh? Oh, no, it's quite small, actually... If anything, too small... |
| Perl Dr. | Too small... Hmmm... |
| JAPH | It's called Update, and... |
| Perl Dr. | I see. And when you run perl -c Update.pm you get errors, right? |
| JAPH | Oh, no, no errors at all Doctor, but... |
| Perl Dr. | ...warnings? |
| JAPH | N-no, no warnings either, but... |
| Perl Dr. | ...run-time errors, then. |
| JAPH | No, the module works, but... |
| Perl Dr. | (somewhat exasperated)...but it takes too long, is that it? |
| JAPH | No, but... |
| Perl Dr. | ...eats up too much memory? |
| JAPH | No, it works perfectly, but... |
| Perl Dr. | (with ill-concealed irritation) Congratulations, Mr. Monk! But let's get back to the reason for your visit, shall we? |
| JAPH | Well, you see, it's this module Update I was telling you about... |
| Perl Dr. | But by your description this module is in perfectly good health, Mr. Monk! |
| JAPH | The thing is... this module is very specialized... |
| Perl Dr. | How so, Mr. Monk? |
| JAPH | Well, hmmm, you see, Update exports only one subroutine... |
| Perl Dr. | Oh, there's nothing wrong with that, Mr. Monk! |
| JAPH | ...called update... |
| Perl Dr. | Hmm, Update::update... Did you ever read Lolita, Mr. Monk? |
| JAPH | Lo—what? |
| Perl Dr. | Never mind, never mind... (Brief pause) Well, certainly, Update::update is not a terribly imaginative choice of names, but it's nothing to send you to the || for! (Winks) |
| JAPH | Yes, but, you see, Update::update does some very specialized things, and... |
| Perl Dr. | Hmph. OK. Let's take a look at this Update::update, Mr. Monk... |
| JAPH |
sub update {
my ( $url, $dir ) = @_;
my @possible_suffixes = qw( .gz .bz2 );
my $basename = extract_basename( $url, @possible_suffixes );
my $local_file = "$dir/$basename";
|
| Perl Dr. | (mumbling) Takes a URL and a directory as args, m-kay... Extracts a basename from the URL, mm-kay... Oh, I see, maybe the URL points to a compressed file, mmm-kay... Generates a local filename from the basename and the input dir, mmmm-kay... |
| JAPH | return if -e $local_file and not is_newer( $url, $local_file );
|
| Perl Dr. | (mumbling) ...returns undef if a local version of the file exists and the remote version is not newer, good, good... |
| JAPH | my $temp_file = download( $url, TEMP_DIR ) or die "download of $url
+failed";
|
| Perl Dr. | (mumbling) ...downloads remote file to a temporary location, very good,... |
| JAPH | if ( -e $local_file ) {
my $new_name = rename_file( $local_file, suffix( time ) );
compress( $new_name );
}
|
| Perl Dr. | (mumbling) Renames the local copy, if any, mmmmm-kay... Oh, I see, the new name has an extension based on the current time... mmmmmm-kay... Compresses the renamed local copy, fine... |
| JAPH | move_file( $temp_file, $local_file );
maybe_uncompress( $local_file );
|
| Perl Dr. | (mumbling) Moves the downloaded file to its final destination, and uncompresses it if necessary, all good... |
| JAPH |
return 1;
}
|
| Perl Dr. | (mumbling) ...returns 1 upon successful completion... mmm-kay! It looks fine, Mr. Monk! What are you concerned about? |
| JAPH | (increasingly agitated) But don't you see what I mean?? It's so specialized, and... |
| Perl Dr. | OK, OK, Mr. Monk, I see that Update::update is a somewhat specialized little function, but let's not jump to conclusions. Why don't you tell me more about Update::update's role in your life? |
| JAPH | I use it to download data files from various sites; these data files are then used to generate secondary data files that are in turn used by several of my CGI scripts. I have these maintenance scripts, you see, run nightly by cron, they all use Update::update... |
| Perl Dr. | It seems quite suitable for the task, Mr. Monk! So what if Update::update is a little specialized... I still don't understand the reason for your... |
| JAPH | IT'S NOT NORMAL! |
| Perl Dr. | Now, now, calm down, Mr. Monk. This is nothing to be upset ab— |
| JAPH | Every module I know of gets used all over the place, in different settings, but since I wrote it three years ago, Update has been used only in those few scripts run by cron every night, all very similar to each other, and... and... |
| Perl Dr. | Re-lax, Mr. Monk. Let me reassure you, there is nothing... |
| JAPH | (visibly conflicted) But don't you think it's weird to use a module here? Shouldn't I be using a script to do all this, and call this script from other scripts using system? |
| Perl Dr. | (continuing unperturbed)...to worry about. Your module is working fine. It is producing the desired results. You should be happy! |
| JAPH | (mumbling) Yes, but... |
| Perl Dr. | (leading JAPH to the door) I suggest that you put this out of your mind, my friend. Go home, have a good night sleep. Just relax, mmm-kay? Everything will be fine. |
| JAPH | OK, I will, ...but — |
| Perl Dr. | Have a nice day, Mr. Monk. |
| JAPH | (as the door closes)...but I want a SECOND OPINION! |
curtain
Erratum extirpated. Thanks to the gimlet-eyed Dr. Zaxo.
Update 2: s/Dr. Perl/JAPH/ on one of the lines. Thanks to ambrus and castaway.
|
Re: To script or not to script, that is the question...
by Zaxo (Archbishop) on May 31, 2005 at 07:01 UTC
|
You need a new Perl Quack, you have an abcessed qw:
my @possible_suffixes = qw( '.gz', '.bz2' );
Lets lance that and give you some antibiotics,
my @possible_suffixes = qw( .gz .bz2 );
There, the swelling's down. Feel better now?
| [reply]
[d/l]
[select] |
|
|
| [reply] |
Re: To script or not to script, that is the question...
by dragonchild (Archbishop) on May 31, 2005 at 12:38 UTC
|
The First Goal of Programming is "An application that works." If you have achieved that, then you have achieved more than 80% of all software projects.
It sounds like "It just works." Nothing wrong with that. There's code that keeps the lights on that hasn't been looked at in 30 years. If it works, then it ... well, works! And, if you don't need to, then DON'T CHANGE IT. Fiddling for the sake of fiddling is a good way to learn where all the food banks in your neighborhood are.
Now, if you have a need to do so, then I would look at improving the name and maybe adding some logging or something of that nature. (Only after correct source control, test suites, and other refactoring infrastructure is in place, though.)
- In general, if you think something isn't in Perl, try it out, because it usually is. :-)
- "What is the sound of Perl? Is it not the sound of a wall that people have stopped banging their heads against?"
| [reply] |
Re: To script or not to script, that is the question...
by tilly (Archbishop) on May 31, 2005 at 17:30 UTC
|
The Perl Dr saw a function named Update::update and thought this was fine??
You need that second opinion!
There are a million kinds of updates out there. If I run across a function named update, I have no idea what it does, and I'll curse the person who wrote it with no mercy for the maintainance programmer looking for context.
Please name it something that gives a hint what it does. Something like "update_download" or "make_download_current".
While you're at it, I would leave out the superfluous "return 1". (Are you using that return anywhere?) | [reply] |
|
|
I concur with tilly. My diagnosis is that your module needs a more specifically named namespace, and possibly a more illustrative name for your subroutine. Weirdly enough, coming up with a perfect name for a subroutine/package/module sometimes takes me longer than the design login and implementation.
So, take two rename update's and call us in the morning. :)
| [reply] |
|
|
The return value of Update::update is used by the calling code to decide whether there are newly updated primary data files that need reprocessing. This doesn't seem unreasonable to me, though I'm open to second opinions on how best to do this, too.
| [reply] |
|
|
That seems reasonable then.
| [reply] |
Re: To script or not to script, that is the question...
by castaway (Parson) on May 31, 2005 at 12:28 UTC
|
Heh.
You name two extremes though, you have a module, complete with own namespace etc, and are trying to decide between that and system()ing a script? The module/own namespace gives you the assurance that you can even use it in scripts that may already have some sort of "update" function that does something else. The other alternative would be to make it a simple .pl containing the sub, and using do "update.pl" in your scripts, which will give the calling script it the update sub in it's own namespace.
C. | [reply]
[d/l] |
Re: To script or not to script, that is the question...
by wazoox (Prior) on May 31, 2005 at 12:09 UTC
|
Well, I'd really like to try this module and give you a second opinion after all :) | [reply] |
Re: To script or not to script, that is the question...
by KPeter0314 (Deacon) on May 31, 2005 at 17:20 UTC
|
Well, every module has to start somewhere and having only one subroutine is a start. Perhaps as time goes on you will come up with an Update::MyMemory, Update::GroceryList, or Update::Something_Else, would you feel better about it then? Or, maybe, this subroutine will never have any siblings.
| [reply] |
|
|
