Beefy Boxes and Bandwidth Generously Provided by pair Networks
Clear questions and runnable code
get the best and fastest answer
 
PerlMonks  

Bad Module Code

by CodeJunkie (Monk)
on Jul 11, 2003 at 15:59 UTC ( [id://273435]=perlquestion: print w/replies, xml ) Need Help??

CodeJunkie has asked for the wisdom of the Perl Monks concerning the following question:

Hello Again
, I'm just attempting to write a module and was wondering if it is bad practise to have subroutines within the module that don't *return* anything? For example, I have a common subroutine I use all the time called printFile(); It goes like this:

sub printFile { my ($file) = @_; open FH, "<$file" or return; print while (<FH>); close FH; # do not complain if it cannot }

Now I want to include this in my module so I can use it as and when I like in my programs, but is this bad coding practice? (I know I should be using a templating system also, but that's a whole other story :-) )

Many thanks,
Tom

Replies are listed 'Best First'.
Re: Bad Module Code
by tilly (Archbishop) on Jul 11, 2003 at 16:06 UTC
    And what do you do when some stupid reason (eg a missing directory, write permissions) causes the files to not open and therefore for nothing to get written?

    Not returning anything is reasonable. No error checking either is bad practice.

      Yes I totally agree with the principle of error checking obviously, but in this case I don't want it to die if the file is missing or it can't be printed. It is just printing HTML markup out to the browser so if it fails it should be reasonably obvious.

      Although having said that, it would be quite nice to print out a message explaining why it didn't work. So I will fix that.

      Cheers,
      Tom

        No, printing would be the wrong thing. Setting $@, using warn() (or something from Carp, throwing an exception would be better. But certainly don't print().

Re: Bad Module Code
by dragonchild (Archbishop) on Jul 11, 2003 at 16:02 UTC
    I do that all the time. I have a wrapper around IO::File that provides a few more option (like when to check for existence and the like), plus throws errors for Error. The closing function doesn't actually return anything.

    Personally, though, I'm anal and always return 1 from functions that are auto-successful. But, that's more from superstition than any real reason.

    ------
    We are the carpenters and bricklayers of the Information Age.

    Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

    Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.

Re: Bad Module Code
by sauoq (Abbot) on Jul 11, 2003 at 18:04 UTC
    I'm just attempting to write a module and was wondering if it is bad practise to have subroutines within the module that don't *return* anything?

    It's actually pretty hard to write a Perl subroutine that doesn't return anything. For example, you might be surprised to learn that your sub returns 1 on success and undef if either of your system calls, open() or close(), fails.

    Without an explicit return, the sub will return the value of the last expression evaluated. As luck would have it, close() will return 1 if it succeeds, so your sub will too.

    When you use an empty return statement, the sub will return either undef or an empty list depending on the context in which it was called.

    Perl only really lets you return nothing when the sub is called in void context (and a return value wouldn't be used anyway.)

    -sauoq
    "My two cents aren't worth a dime.";
    
      In perl's spirit of DWIM (Do What I Mean) and its design for doing what comes naturally, your function (recopied below) may now *return* anything while returning a lot of useful information:
      printFile($file) or warn "Could not printFile, '$file': $!"; sub printFile { my ($file) = @_; open FH, "<$file" or return; print while (<FH>); close FH; # do not complain if it cannot }

      Thanks to the design you will get a meaningful (undef) return if either the open or close fail. Aah, I love this language It does what you want it to do even when you don't think you told it to do anything. It's like perl can read my mind.
      $will->code for @food or $$;
Re: Bad Module Code
by blue_cowdawg (Monsignor) on Jul 11, 2003 at 16:03 UTC

    Well... It is probably a bad idea to have subs anywhere that don't return values for at least some form of "I was able to do that" or "bad things happened to me" kinds of flags to the calling environment.

    This is regardless of if you are writing a module or simply a procedure oriented piece of code.

    Having said that I violate my own rule all the time when I am coding in a hurry.


    Peter L. BergholdBrewer of Belgian Ales
    Peter@Berghold.Netwww.berghold.net
    Unix Professional
      Well, that depends. Some things may have useless return calls, such as those that try and push out a command.

      take, say.. something that initiates garbage collection, or flushes buffers. You can't really say it succeeds or fails other than the fact that you've told it to run or not.

      I readily admit, there could be a return value if the request for the subsystem to run is fine, but sometimes, you just don't know. Look at sun's sync function for instance :)

        look at sun's sync function for instance :)

        Please... I just ate... :-)


        Peter L. BergholdBrewer of Belgian Ales
        Peter@Berghold.Netwww.berghold.net
        Unix Professional
Re: Bad Module Code
by del (Pilgrim) on Jul 11, 2003 at 17:27 UTC
    That subroutine could return a value, though. The last returned value within the subroutine will be the return value of the subroutine.
    perldoc -f close
    " Closes the file or pipe associated with the file handle, returning true only if stdio successfully flushes buffers and closes the system file descriptor. "
Re: Bad Module Code
by ajdelore (Pilgrim) on Jul 11, 2003 at 19:37 UTC

    In the case of a single program or script, I would agree that it is unimportant that your function return anything if this is not needed.

    However, since this is a module, and intended for re-use, the function may be used in other situations that you might not anticipate.

    In this case, I believe that any function should return at minimum a 1 or 0 to indicate success/failure. Of course, as others have very astutely pointed out, your function does this anyways.

    </ajdelore>

    OT beef: How may vi users out there press Esc while editing text in the textarea field and lose everything they wrote? Grrr...

Re: Bad Module Code
by Anonymous Monk on Jul 11, 2003 at 16:11 UTC
    What you could do is wrap the contents of your sub in an eval block and return with an error code to the caller if eval saw an error, sort of like try/catch.
Re: Bad Module Code
by bobn (Chaplain) on Jul 11, 2003 at 16:57 UTC

    Don't forget that if called as a method, the first parm will be the invocant (object).

    --Bob Niederman, http://bob-n.com
Re: Bad Module Code
by ihb (Deacon) on Jul 12, 2003 at 15:18 UTC
Re: Bad Module Code
by exussum0 (Vicar) on Jul 11, 2003 at 20:47 UTC
    Actually, die shouldn't return anything. Nor should confess :) HA!
Re: Bad Module Code
by runrig (Abbot) on Jul 12, 2003 at 01:35 UTC
    I'm just attempting to write a module and was wondering if it is bad practise to have subroutines within the module that don't *return* anything?
    But your function is returning something. It is returning false if there is an error opening or closing the file, and true otherwise. Whether or not you use the return value is another matter...
Re: Bad Module Code
by QwertyD (Pilgrim) on Jul 13, 2003 at 20:04 UTC

    Whether your sub actually returns anything to the caller is one thing, but whether its return value means anything is another.

    In the module's documentation, it's fine to either leave out a mention of what the sub returns, or explicitly say that what the sub returns is unspecified. This tells anybody using your sub that they should not rely on anything it returns. This way, in a later version of the module, if you decide that you want to return true, or a status code, or some kind of error message, you can, and you won't break any existing code.

    For a little more on this, see the "Specifying semantics" section of Ned Batchelder's essay on interfaces. He uses the term "undefined" instead of "unspecified", but that's just because the languages he usually programs in probably don't have undef.


    How do I love -d? Let me count the ways...

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://273435]
Approved by dragonchild
Front-paged by dragonchild
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others musing on the Monastery: (3)
As of 2024-03-19 11:54 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found