Why isn't this function writing anything to my file in Perl?

Question

I'm relatively new to manipulating files in Perl. I don't know what's wrong with this code that it does not write anything to my text file. I don't think it's with my data structure because I have another function which prints the contents of the data structure and the data is there. It just doesn't get written in the file. Am I missing something?

Here is the code :

sub saveFile {
    open( my $out, ">", "inputs.txt" );
    for ( $i = 0; $i < $#students; $i++ ) {
        print $out $students[$i]->{"name"};
        print $out $students[$i]->{"studNum"};
        print $out $students[$i]->{"cNum"};
        print $out $students[$i]->{"emailAdd"};
        print $out $students[$i]->{"gwa"};
        print $out $students[$i]->{"subjNum"};
        for ( $j = 0; $j < $students[$i]->{"subjNum"}; $j++ ) {
            print $out $students[$i]->{"subj"}->[$j]->{"courseNum"};
            print $out $students[$i]->{"subj"}->[$j]->{"courseUnt"};
            print $out $students[$i]->{"subj"}->[$j]->{"courseGrd"};
        }
    }
    close $out;
    print "FILE SAVED.\n";
}

Do you have `use strict; use warnings;`? If not, add it and fix the warnings. You should also avoid the global variable and instead pass it as an argument to your function. — TLP, Sep 14 '14 at 13:21
Instead of typing out `$students[$i]->{"subj"}->[$j]->{"courseNum"}` when using hash references, you only need to use one arrow: `$students[$i]->{"subj"}[$j]{"courseNum"}`. — i alarmed alien, Sep 14 '14 at 13:38
Always check the return code from `open` and take appropriate action. `open my $out, '>', 'inputs.txt' or die "Can't open inputs.txt: $!\n";` — Dave Cross, Sep 15 '14 at 09:43

score 4 · Answer 1 · edited Sep 14 '14 at 19:22

You have used < where you should have used <= in your loop condition.

for ( $i = 0; $i < $#students; $i++ ) {
#                ^^-- here

Because of this, the last index will not be printed, and I assume you are testing this using just one record.

Note that this is a poor way of looping over an array. A much preferred way -- unless you need the array indexes for something else -- is to loop over the actual elements:

for my $student (@students) {
    print $student->{"name"};
    ...
}

Each element here will be a hash ref, which will simplify your typing considerably and make your code more readable.

You can also use a hash slice to print your records, instead of printing them one by one:

print @{$student}{qw(name studNum cNum emailAdd)};

Note that you may want to delimit the various values with something:

print join ",", @{$student}{qw(name studNum cNum emailAdd)};

And lastly, like I said in the comments, you should avoid using a global variable. Instead pass the array to your function. Combined with all my advice, I came up with this:

saveFile( \@students );

...;
use feature 'say';    # required for say()

sub saveFile {
    my $aref = shift;
    for my $student (@$aref) {
        say join ",", @{$student}{qw(name studNum cNum emailAdd gwa subjNum)};
        for my $subj ( @{ $student->{"subj"} } ) {
            say join ",", @{$subj}{qw(courseNum courseUnt courseGrd)};
        }
    }
}

Note that I have done away with the subjNum key, which is redundant, as it is simple to get the size of the array.

thb · Accepted Answer · 2014-09-14T13:51:16.233

2

Though your source formatting wants improvement, your technique is basically right. However, there is this little error in your code:

$i<$#students

Make it either $i<=$#students or $i<@students. That should fix your trouble.

By the way, if you wish to improve formatting (and idiom), then this alternative might afford you some ideas:

sub saveFile {

    open(my $out, '>', 'inputs.txt');

    for (my $i = 0; $i <= $#students; ++$i) {

        print $out $students[$i]{name};
        print $out $students[$i]{studNum};
        print $out $students[$i]{cNum};
        print $out $students[$i]{emailAdd};
        print $out $students[$i]{gwa};
        print $out $students[$i]{subjNum};

        for (my $j = 0; $j < $students[$i]{subjNum}; ++$j) {
            print $out $students[$i]{subj}[$j]{courseNum};
            print $out $students[$i]{subj}[$j]{courseUnt};
            print $out $students[$i]{subj}[$j]{courseGrd};
        }

    }

    close $out;
    print "FILE SAVED.\n";

}

edited Sep 14 '14 at 13:51

answered Sep 14 '14 at 13:29

thb

13,796
3
40
68

1

Oh i see. Didn't see that. I just realized that the variable mentioned tracks the last element of the array, not the next after it. Thank you! – ejandra Sep 14 '14 at 13:37
Using an empty argument list in a prototype is redundant and just complicates things. `sub saveFile { }` is what you should use. – TLP Sep 14 '14 at 13:40
1

@thb OP could also eliminate the extra arrows in the hashrefs, and simple `for (@$students)` and `for (@{$students[$i]})`. – i alarmed alien Sep 14 '14 at 13:42
@TLP: I did not know that an empty argument list complicated things, for I have been using empty argument lists for many years. I will remove the empty argument list from my answer, but would you tell me why? I would like to learn. – thb Sep 14 '14 at 13:43
See http://stackoverflow.com/questions/297034/why-are-perl-5s-function-prototypes-bad – i alarmed alien Sep 14 '14 at 13:44
2

@tbh Prototypes in Perl are different from other languages. They are only there to allow advanced programmers ways to emulate built-in functions, to provide extra functionality to argument handling. When you use prototypes, Perl will expect your function to use a certain amount and type of arguments, and will warn about missing arguments, and whether you do not predeclare your function. The general rule is: If you don't know what prototypes do in Perl: Don't use them. Which is short for: Don't use prototypes. – TLP Sep 14 '14 at 13:52
@nowrn29: StackOverflow allows you to upvote answers you find helpful. – thb Sep 14 '14 at 13:56

Why isn't this function writing anything to my file in Perl?

2 Answers2