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

I'm trying to write a simple logger class (code below) ... when I create more than one object from another Perl file, if I call ->message the text ends up in the correct file, if I call ->finish everything ends up in one file. I guess my mistake is to have created just one filehandle, but I can't spot what I'm doing wrong or where. I'd be grateful if someone can point out where I'm going wrong. Thanks
sub new { my $class_name = shift; my $self = {}; $output_file = shift; $append = shift; $file_handle = undef ; bless ($self,$class_name); bless ($self); return $self; } # opens log file for writing # (no argument) will be called from first call to &message if not call +ed from outside sub start{ my $now = &get_timestamp; print "opening $output_file\n"; if (uc($append)eq'APPEND') { open ($file_handle,">>$output_file") || open ($file_handle,"> +$output_file") || die "Can't open $output_file $!"; }else{ open ($file_handle,">$output_file") || die "Can't open $output +_file $!"; } print $file_handle "Started logging at $now\n"; print $file_handle "Logging to $output_file\n"; print $file_handle "\n"; } #log a message # argument <a status>, <a text message> sub message{ #run start subprocedure if file not already open &start if (!$file_handle); my $status = $_[1]; my $text = $_[2]; print $file_handle "$status : $text\n" ; #$message_number++; } #close log # (no argument) sub finish{ #run start subprocedure if file not already open if (!$file_handle){ &start; }else{ my $now = &get_timestamp; print $file_handle "\n"; print $file_handle "Finished logging at $now\n"; print $file_handle "-------------------------------------\n"; #close ($file_handle); } }

Replies are listed 'Best First'.
Re: creating 2 file writing objects
by vladb (Vicar) on Jun 27, 2002 at 13:54 UTC
    I'd be grateful if someone can point out where I'm going wrong

    Brother postman, since you have displayed interest in our (my) advice, let me suggest to you that it's always better to use modules that are already on the CPAN other than re-invent your own wheel. The advantage of this approach is a plenty. First, CPAN modules are generally well (or sufficiantly) tested. Second, you don't have to do any work re-implementing the tool/functionality.

    As for specific module suggestions, check out these ones:
    • Log::Agent
      The module provides an abstract layer for logging and tracing.

    • Log::Channel
      Allows for code to specify channels for delivery of logging messages, and for users of the code to control the delivery and formatting of the messages.

    • Log::Simple
      This is a basic run-time logger. My favourite ;).


    UPDATE:

    Also, as far as your class code goes, you aren't doing proper OO implementation. The $filehandle should be stored 'inside' your object. So, your constructor should look as follows:
    sub new { my $class_name = shift; my $self = {}; $self->{output_file} = shift; $self->{append} = shift; $self->{file_handle} = undef ; bless ($self,$class_name); # bless ($self); # AHA! How could I have missed that one! +Thanks go to [bronto] for pointing that out. No need to bless the thi +ng twice.. one time is enough already! ;> return $self; }
    Further, the rest of the methods in your modules aren't even true 'member' methods as they don't make use of the object 'reference'. In Perl, you've got to link a method to an object as so:
    sub start { my $self = shift; # Get 'reference' to the class instance (aka obje +ct)! # use this reference to retrieve object specific attributes! open($self->{filehandle}, " . . . "); # . . . Rest of your code . . . }


    Update: bronto thanks for pointing that out. However, the last portion of your statement is invalid:
    but I don't see the need to bless $self twice, an error that you inherited from vladb's code

    I didn't make any errors and therefore noone could have 'inherited' any from me. I simply copied and pasted the original poster's code and overlooked that he (the original author, not I) called the 'bless' method twice.

    _____________________
    # Under Construction

      I subscribe the advice of using CPAN modules and of putting the filehandle inside the object, but I don't see the need to bless $self twice, an error that you inherited from vladb's code

      Update: s/[vladb]/[postman]/

      Sorry, wrong target ;-)

      Ciao!
      --bronto

      # Another Perl edition of a song:
      # The End, by The Beatles
      END {
        $you->take($love) eq $you->made($love) ;
      }

Re: creating 2 file writing objects
by Joost (Canon) on Jun 27, 2002 at 14:02 UTC
    You are not storing your user object data in your object and you are not calling methods as methods but as subroutines, using the &routine syntax that has some really nasty side-effects.

    Some pointers:

    use strict; would have kicked your ass :-)

    Your constructor should be something like:

    use Symbol; sub new { my $class_name = shift; $classname = ref($classname) || $classname; my $self = { "output_file" => shift, "append" => shift, "file_handle" => gensym(), } bless ($self,$class_name); return $self; }

    use Symbol; and gensym() are put in here for backward compatibility to perls before 5.6 - see the Symbol manpage

    And methods should be called and constructed like:

    $object->method($arg1,$arg2); sub method { my $self = shift; my $arg1 = shift; my $arg2 = shift; print "append is ",$self->{"append"},"\n"; $self->other_method($arg2); }

    Take a look at perldoc perltoot and/or get your hands on the 'programming perl' book

    Update: changed $append and friends to "append" - that should actually work :-)

    also added some code to show the retrieval of object data.

    -- Joost downtime n. The period during which a system is error-free and immune from user input.
      Many thanks to you ... and the others who replied so quickly, for pointing out the error of my ways which stems in part I think from trying to write Java in Perl. I could just use a cpan module, but I had the feeling that I was misunderstanding something fundamental that I needed to sort out. Next on my list is to re-read perltoot. Meanwhile ... one more question if I may, I'm trying to
      print $self->{file_handle} "some text \n" ;
      which is wrong, but can't figure out what it should be. Thanks again
        On top of my OO confusion, I was getting led further astray by my overlooking the global nature of filehandles, and my mistaken way of trying to store them in a hash. Got there in the end ... by storing a reference to the file handle in a hash
        use FileHandle; my $FH = FileHandle->new(">>$self->{output_file}"); $self->{file_handle} = \$FH;
        and used in other methods like this
        my $FH = ${$self->{file_handle}}; print $FH "some text\n" ;
        Thanks again for all the pointers.