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

Perl is my first programming language. I wrote this to play with pattern matching and filehandles. Please could someone explain why this code does everything I ask except print to the screen a count of the number of instances of the word I'm substituting? Massive thanks.
#! c:\perl\bin print("Enter filename to open (text only):\n\n"); $in=<STDIN>; chomp $in; if ($in !~ /\.txt$/i) { $in.=".txt"; } open ($in, "$in") or die("$in could not be opened:$!\n"); print("Enter string to replace \"and\" with:\n\n"); $word=<STDIN>; chomp $word; print("Enter filename to SAVE text file as:\n\n"); $out=<STDIN>; chomp $out; if ($out !~ /\.txt$/i) { $out.=".txt"; } open($out, ">$out") or die $!; print ("\n\n\n"); $count=0; while (<$in>) { s/\band\b/$word/ig; while (/\band\b/ig) { ++$count; } print($out "$_\n"); } print "there were $count instances of \"and\" in $in"; close $out; close $in;
UPDATE. Thanks everyone! Most helpful.

Replies are listed 'Best First'.
Re: Counting instances of words
by philcrow (Priest) on May 25, 2007 at 16:22 UTC
    You are attempting to count occurances of and AFTER you've already made the substitutions.

    Others may give advice on coding style, I'm off to lunch.

    Phil

    The Gantry Web Framework Book is now available.
Re: Counting instances of words
by blazar (Canon) on May 25, 2007 at 16:39 UTC
    #! c:\perl\bin print("Enter filename to open (text only):\n\n");

    No

    use strict; use warnings;

    Too bad!

    $in=<STDIN>; chomp $in; if ($in !~ /\.txt$/i) { $in.=".txt"; }
    chomp(my $in=<STDIN>); $in .= '.txt' unless $in =~ /\.txt$/i;
    open ($in, "$in") or die("$in could not be opened:$!\n");

    Too bad: not only it features useless quoting of variables, but it's also wrong in that for lexical filehandles to work, the variable must be undefined.

    open my $infh, '<', $in or die "$in could not be opened:$!\n";
    s/\band\b/$word/ig; while (/\band\b/ig) { ++$count; }

    After you've substituted all occurrances of 'and' with $word, there won't be many remaining unless $word =~ /\band\b/i. Count around the substitution, instead.

Re: Counting instances of words
by citromatik (Curate) on May 25, 2007 at 16:32 UTC
    The problem is here:
    while (<$in>) { s/\band\b/$word/ig; # <- Here you make all the substitutions while (/\band\b/ig) # <-... and here nothing happens! { ++$count; } print($out "$_\n"); }

    s/// will return the number of occurrences, so...

    while (<$in>) { $code = s/\band\b/$word/ig; # <- Here you make all the substit +utions ## UPDATE: ## $code += s/\band\b/$word/ig; # <- Here you make all ## Sorry for the mistake (thanks blazar!) print($out "$_\n"); }

    should works

    citromatik

      Thanks for your help. Tried this, and $code appears to be empty. I've declared it outside the While loop, but it prints out as null. Should this contain the total substitutions?
        Thanks for your help. Tried this, and $code appears to be empty. I've declared it outside the While loop, but it prints out as null. Should this contain the total substitutions?

        Well, for one thing I wouldn't regard $code as a particularly good variable name. Then, if you want the total number of substitutions, you must accumulate them:

        $count += s/\band\b/$word/ig;
Re: Counting instances of words
by strat (Canon) on May 28, 2007 at 11:38 UTC

    Hi scarymonster,

    Perl is a very flexible and powerful language, even powerful enough to shoot away your own foot, and perl won't complain. But there exists a perl-builtin pragma with name use warnings; which tries to warn you if you write code that might be problematic and to warn you from some errors that may be difficult to be found. If you dont understand what such a warning means, also use diagonstics; which will try to explain why this might be dangerous.

    Another very big help is use strict; which may save you from a lot of other - difficult to find - errors. Why you should use strict is a very good node to start with, and Use strict and warnings and 101 reasons to use strict and Why We Use Strict and Use strict warnings and diagnostics.

    So it's a good idea to start every program with the following lines:

    #! /usr/bin/perl use warnings; use strict; use diagnostics;

    Here some stuff for security:

    You don't control if the filenames given are really filenames, and not commands that will be executed. Try the following code which will try to open notepad under windows:

    use warnings; use strict; my $filename = 'notepad |'; open( my $FH, $filename ) or die "Can't open '$filename': $!\n";

    You can easily avoid this if you use the open with three parameters:

    use warnings; use strict; my $filename = 'notepad |'; open( my $FH, "<", $filename ) or die $!;

    And if you want to open an existing file for reading, it might also be a good idea to check if it is really existing before opening it:

    my $filename = 'notepad |'; unless( -f $filename ) { die "Error: file '$filename' is not existing\n"; } # unless

    e.g

    #! /usr/bin/perl use warnings; use strict; use diagnostics; print "Enter filename to open (text only):\n\n"; my $infile = <STDIN>; chomp $infile; unless( -f $infile ) { die "Error: file '$infile' is not existing\n"; } open( my $INFILE, '<', $infile ) or die "Error: can't open '$infile' for reading: $!\n"; print "Enter filename to SAVE text file as:\n\n"; my $outfile = <STDIN>; chomp $outfile; open( my $OUTFILE, '>', $outfile ) or die "Error: can't open '$outfile' for reading: $!\n"; print qq~Enter string to replace "and" with\n\n~; my $word = <STDIN>; chomp $word; print "\n" x 3; my $count = 0; while( <$INFILE> ) { $count += s/ \band\b /$word/gix; print $OUTFILE $_; } # while close $INFILE or die "Error: can't close '$infile' for reading: $!\n"; close $OUTFILE or die "Error: can't close '$outfile' for writing: $!\n"; print qq~There were $count instances of "and" in '$infile'\n";

    Best regards,
    perl -e "s>>*F>e=>y)\*martinF)stronat)=>print,print v8.8.8.32.11.32"