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

Hi, The script below doesn't print anything.I just modified the code to add to two functions "warning_entry" and "error_entry".I want them to put in a switch case and based on the user input I want I one of them to execute but currently the below script doesn't print anything.Can someone pls help on how to add a switch case and why is nothing get printed?

#!/usr/bin/perl -w use strict; use warnings; use Spreadsheet::WriteExcel; use List::MoreUtils qw( uniq ); open my $line, '<', 'file.log' or die 'Cannot open file'; my $cr_line; my $i = 1; my ($is_an_error_entry,$is_a_warning_entry); my $resolution_link; my (%seen_cr,%seen_match,%seen_link) ; my ($crline,$Buggyfile,$resolutionlink,$match); #subroutine for errors sub error_entry { print "in error entry";#not getting printed my $workbook = Spreadsheet::WriteExcel->new("errors.xls"); my $worksheet = $workbook->addworksheet("Error"); if ($match =~ /Error:/) { $is_an_error_entry = 1; next LINE; } if ($match =~ /https?:/) { if ($is_an_error_entry) { ($resolution_link) = $match =~/(https?:\S+?)[.]?\s/; } } } #subroutine for warnings sub warning_entry { my $workbook = Spreadsheet::WriteExcel->new("warnings.xls"); my $worksheet = $workbook->addworksheet("warnings"); if ($match =~ /Warning:/) { $is_a_warning_entry = 1; } if ($match =~ /https?:/) { if ($is_a_warning_entry) { ($resolution_link) = $match =~/(https?:\S+?)[.]?\s/; } } # -------------------------------------------------------------------- +---- # MAIN SCRIPT # # -------------------------------------------------------------------- +---- while ( $match = <$line> ) { if ( $match =~ /CR:/ ) { $cr_line = (split /:\s*/, $match)[1]; chomp $cr_line; $is_an_error_entry = 0; } #need a switch here #1.error entry #2.warning entry print "DEBUG";#no gettting printed error_entry(); warning_entry(); if ( $match =~ /Perforce path:/ ) { if ($is_an_error_entry) { $match = (split /:\s*/, $match)[1]; chomp $match; if ( ($cr_line) and ( !$seen_cr{$cr_line}++ ) ) { $crline = $cr_line; } print $crline, $match, $resolution_link, "\n"; $worksheet->write( $i, 0, $crline ); $worksheet->write( $i, 1, $match ); $worksheet->write( $i, 2, $resolution_link ); $i++; } $cr_line = ''; $crline = ''; $resolution_link = ''; } } }
  • Comment on Why is my script not printing anything?and how to add a switch case in perl
  • Download Code

Replies are listed 'Best First'.
Re: Why is my script not printing anything?and how to add a switch case in perl
by GrandFather (Saint) on Dec 02, 2010 at 03:30 UTC

    This is a lovely example of why global variables are a bad idea! By making all your variables global you almost completely obviate the primary reason for using strict. Don't use globals like that!

    As toolic observed your rubbishy indenting hides the fact that you have nested your 'main' code inside one of your subroutines. A big comment in the code does nothing at all to fix that!

    Keep variables local and pass them around to subroutines if required. Id' provide sample code to show how this could be tidied up, but there's not enough context for me to be sure just what you are parsing and how you really want to handle errors. If you provide more context (a small sample input file and expected output would help) someone might provide a nice example for you to work from.

    True laziness is hard work

      Is there a way I can attach files to my replies?I can attach sample input and output files?

      Enter your choice: 1.Errors 2.Warnings

      Would the following sample switch code work when the user is given the above choices?

      given($_) { when (/1/) { error_entry(); } when (/2/) { warning_entry(); } }

        You can't attach files, but you can include the contents of text files in your reply by putting the text in code tags. Please though keep the size of such samples small - just enough to demonstrate the issue. Often that will mean creating suitable sample data (perhaps by trimming down a real file).

        I'd write that as:

        given($answer) { when ('1') {error_entry();} when ('2') {warning_entry();} default {die "Expected 1 or 2 for the answer. Got $answer\n";} }

        Using a regex match would match on any answer containing a 1 digit for the first case and any answer containing a 2 digit for the second case. Maybe not a problem for your application, but as a general thing you should validate data and handle unexpected data in some appropriate fashion. die is most appropriate for production code when you are using it to throw exceptions that are caught by eval. Generally it is much better to die and have it handled (or not in which case to script does die) than to carry on processing bogus data.

        True laziness is hard work
Re: Why is my script not printing anything?and how to add a switch case in perl
by toolic (Bishop) on Dec 02, 2010 at 02:27 UTC
    Your inconsistent indentation makes it hard to follow your code (even when I run it through perltidy), but I suspect you are never calling either function. I think your "MAIN SCRIPT" code is actually inside your warning_entry sub. Try adjusting your curly braces.

    Switch statements