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

Hello monks, I am trying to make a script that will search through and analyze the contents of a log file. I want it to look for the string "clip" and return how many times it occurs to a variable, and print it to an output file, but Perl keeps crashing on me. Surely one of you can help?
$playername = ''; $clipsobt = ''; $done = ''; @logfile = ''; $logfile = ''; open(LOGFILE, "C:\\Documents and Settings\\Alex\\Desktop\\doomlog.txt" +) or die "Can't open DOOM log file: $!\n"; open(LOGWRITE,">C:\\Documents and Settings\\Alex\\Desktop\\explog.txt" +) or die "Can't open log file for writing: $!\n"; until ($playername) { &askname(); } &clipcalc(); print LOGWRITE "You have obtained $clipsobt clips.\n"; sub askname { print "What is your player's name?: "; chomp($playername = <STDIN>); } sub clipcalc { @logfile = split(" ", <LOGFILE>); foreach $logfile (@logfile) { if ($logfile =~ /clip/i) { $clipsobt++ } } }

Replies are listed 'Best First'.
Re: Error Problem
by GrandFather (Saint) on Apr 17, 2008 at 01:52 UTC

    First off - always use strictures (use strict; use warnings;).

    Perl initializes everything for you (to undef mostly) so if you don't have a real value to assign to a variable at declaration time don't assign anything. Combined with use warnings; that often gives early warning about logic errors.

    Declare variables as late as possible so the scope over which they are used is clearer.

    Uber points for checking the result of your opens, but you should use the three parameter open for extra safety and clarity.

    Don't use & for calling subroutines - it's generally not needed and often doesn't do what you think.

    Don't use global variables in subs (askname for example) - it's often unclear where variables are altered if you do that sort of thing leading to difficult to debug problems. Your name fetching loop may be better as:

    my $playername; until ($playername) { print "What is your player's name?: "; chomp($playername = <STDIN>); }

    Apart from a reprise of the globals and calling convention issues, clipcalc only processes the first line of the log file. It would be better written as:

    while (<LOGFILE>) { $clipsobt++ if /\bclip\b/i; }

    Perl is environmentally friendly - it saves trees
Re: Error Problem
by NetWallah (Canon) on Apr 17, 2008 at 01:40 UTC
    clipcalc() is reading only the first record of the file.

    You need to put that in a loop, to read the entire file.

         "How many times do I have to tell you again and again .. not to be repetitive?"

Re: Error Problem
by oko1 (Deacon) on Apr 17, 2008 at 02:00 UTC

    You should 'use warnings' and 'use strict' in your scripts; they'll help you diagnose a lot of problems.

    @logfile = split(/ /, <LOGFILE>);

    The problem with the above is that it's going to treat the <LOGFILE> filehandle as a scalar - that is, it's going to extract one line from it - which is presumably the cause of many of your problems. Try this:

    #!/usr/bin/perl -w use strict; cd "C:\\Documents and Settings\\Alex\\Desktop"; open LOGFILE, "doomlog.txt" or die "doomlog.txt: $!\n"; open LOGWRITE,">explog.txt" or die "explog.txt: $!\n"; my $clipsobt; for my $logfile (<LOGFILE>){ if ($logfile =~ /clip/i) { $clipsobt++ } } print LOGWRITE "You have obtained $clipsobt clips.\n";

    Or you can go with a shorter version:

    #!/usr/bin/perl -w use strict; cd "C:\\Documents and Settings\\Alex\\Desktop"; open LOGFILE, "doomlog.txt" or die "doomlog.txt: $!\n"; open LOGWRITE,">explog.txt" or die "explog.txt: $!\n"; my $clipsobt = grep /clip/, <LOGFILE>; print LOGWRITE "You have obtained $clipsobt clips.\n";

    Update: Darn it, the node got re?aped while I was answering it (I think.) ++ to GrandFather for reading my brain and typing out the answer for me - wish I'd known and saved myself all the typing... :)

    
    -- 
    Human history becomes more and more a race between education and catastrophe. -- HG Wells