2

I am trying to make changes to an existing Perl script. I am trying to add a subroutine that is called many times at different places and prints different output based on where is it called.

Basically, it creates a file and writes some business message for the business team (I am part of and hence my first time coding in Perl).

What I want/am trying for is

Sub filemessage ($arg1) {
  open(fh, '>', $message_file);
  print fh "$arg1/n";
  close fh;
}

Now I want to call this subroutine at many places (inside other subroutines) like this (there is already a fail condition based on which it is called):

filemessage ("failed due to reason1")
filemessage ("failed due to reason2")
0

3 Answers 3

7
Sub filemessage ($arg1) {
  open(fh, '>', $message_file);
  print fh "$arg1/n";
  close fh;
}

You were very close.

In Perl, subroutines are created with the sub keyword (note the lowercase 's').

The newline escape sequence is \n, not /n.

These days, we like to use lexical variables as filehandles (this, among other advantages, will automatically close the file as the variable goes out of scope).

Using > as the file mode will create a new, empty file each time you open the file. So it will only ever contain the last message added. Switching to >> will append the messages.

You should check the return value from your call to open() and take appropriate action if it fails (usually killing the program is the best option).

You should declare and define $message_file somewhere. As it's a bad idea for a subroutine to access external variables, you probably want to do that inside the subroutine.

But your biggest problem is the line:

sub filemessage ($arg1)

Traditionally Perl's subroutine parameters are passed in an array called @_ and you need to extract them yourself.

my ($arg1) = @_;

But since Perl 5.20, there is an experimental signatures feature that works how you expect it to.

Putting all that together, we end up with this:

# turn on signatures (put this with the other "use"
# statements at the top of your file).
use experimental 'signatures';

sub filemessage ($arg1) {
  my $message_file = 'log.txt';

  open(my $fh, '>>', $message_file)
    or die "Can't open [$message_file]: $!\n";

  print $fh "$arg1\n";
} 
Sign up to request clarification or add additional context in comments.

2 Comments

I thought you said he should use append >>. Also, newline in the die message will prevent the error from printing the line number that death occurred, which is very rarely a good idea. I would not recommend a newbie to use experimental features when normal features are so simple to use.
@TLP: The > was a typo that I've now fixed. I know what the newline does and I added it on purpose. In a production situation, I'd probably use croak instead of die as for an often-used subroutine like this, the place where the subroutine is called from is often more useful than the line in the subroutine. I think this is an experimental feature that makes Perl work more how non-Perl programmers expect it to work, so I think it's well worth recommending (plus, I believe it's likely to come out of experimental status soon).
3

Please investigate the following code piece.

Original code required just a little touch of 'love'.

  • you should use >> instead > as the last one will overwrite each sub call
  • it is useful to add timestamp into the message for debug purpose
  • indicate successful program completion with exit code 0 and an error with some specific error code

NOTE: always include use strict; and use warnings; in your code, it will help you to avoid many pitfalls in a code. Perl will warn you when you do something what can lead to an error or potentially unexpected result.

NOTE: by adding $filename as an argument to the subroutine reusability of code piece increases, use same subroutine code to write messages in various files (sometimes very useful for debug purpose)

use strict;
use warnings;
use feature 'say';

my $filename = 'my_prog.log';
my $msg;

$msg = 'INFO: my_prog is started';
logger($filename,$msg);
$msg = 'INFO: my_prog is running';
logger($filename,$msg);
$msg = 'ERROR: my_prog immitage some error';
logger($filename,$msg);
$msg = 'INFO: my_prog is done';
logger($filename,$msg);

exit 0;

sub logger {
        my $file = shift;
        my $msg  = shift;

        open my $fh, '>>', $file
                or die "Couldn't open $file";
        say $fh join(' ', scalar localtime, $msg);
        close $fh;
}

Produces output

Mon May 24 01:23:19 2021 INFO: my_prog is started
Mon May 24 01:23:19 2021 INFO: my_prog is running
Mon May 24 01:23:19 2021 ERROR: my_prog immitage some error
Mon May 24 01:23:19 2021 INFO: my_prog is done

Reference: open, say

Comments

2

Here are a few changes that should make it work:

my $message_file = 'some_file';

sub filemessage {                  # sub is spelled with a lowercase 's'
    my ($arg1) = @_;               # args are passed through the array @_
    open(my $fh, '>', $message_file) or die "Error: $!"; # declare my $fh
    print $fh "$arg1\n";           # \ not /
    close $fh;
}

Note: The mode > will truncate the file so you'll only ever have at most one message in the file. If you want to append to the file, use the mode >>.

7 Comments

Oh, you want >>. You might also want to admonish all those opens+closes, using a flush instead.
@ikegami ">>" - I'm not sure if OP wants one message in the file or append. I'll leave that up to OP but added a note.
Args are passed to $j? Shouldn't that be $_[j]?
Args are not passed as $1, $2, etc.
The code is not doing what it claims; $_[0] should be there instead of $1. (or shift)
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.