Perl subroutines

Question

Here I fixed most of my mistakes and thank you all, any other advice please with my hash at this point and how can I clear each word and puts the word and its frequency in a hash, excluding the empty words.. I think my code make since now.

Answer 1

So you can focus on the key part of the algorithm, how about accepting input on STDIN and output to STDOUT. That way there's no argument checking, etc. Just a simple:

$ prog < words.txt

All you really need is a very simple algorithm:

• Read a line
• Split it into words
• Record a count of the word
• When done, display the counts

Here's a sample program

#! /usr/bin/perl -w    
use strict;

my (%data);
while (<STDIN>) {
    chomp;
    my(@words) = split(/\s+/);
    foreach my $word (@words) {
        if (!defined($data{$word})) {
            $data{$word} = 0;
        }
        $data{$word}++;
    }
}

foreach (sort(keys(%data))) {
    print "$_: $data{$_}\n";
}

Once you understand this and have it working in your environment, you can extend it to meet your other requirements:

• remove non-alphabetic characters from each word
• print three results per line
• use input and output files
• put the algorithm into a subroutine

Answer 2

I agree that starting with dave's answer would be more productive, but if you are interested in your mistakes, here is what I see:

1. You assign the return value of checkArgs to a scalar variable $checkArgs, but return an array value. It means that $checkArgs will always contain 2 (the size of the array) after this call (because the program dies if the number of arguments is not 2). It is not very bad since you do not use the value later, but why you need it at all in this case?

2. You open files and close them immediately without reading from them. Does not make sense.

3. Statement

   while (<>)

reads either from standard output or from all files in the command line arguments. The latter variant is like what you want, but your second argument is the output file, not input. The diamond operator will try to read from it too. You have two options: a) use only one file name in the command line arguments, read the file with <>, use standard output for output, and redirect output to a file in shell; b) use

while(<$file1>)

instead, of course, before closing files. Option a) is the traditional Unix- and Perl-style, but b) provides for clearer code for beginners.

4. Statements

   return $word;

and

return $str, $hash{$str};

return corresponding values on the first iterations of the loops, all other data remain unprocessed. In the first case, you should create a local array, store all $word in it and return the array as a whole. In the second case, you already have such local %hash, it is enough to return this hash. In both cases, you need should assign the return values of the functions not to scalars, but to an array and a hash correspondingly. Now, you actually lose all you data.