Writing contents of an array to txt file in perl

Question

The output I'm trying to achieve would look like this:

Name age gpa

for every element in each array, however when I write the contents of the array to a file it over writes somewhere and is display like this:

name1 name2 age1

Here's my code, all help (even on my coding style) is greatly appreciated. I've only displayed the parts of the code I think are necessary, if more is required for help please let me know.

sub get_name
{
    printf("Name :");
    push(@get_name, <STDIN>); #taking name from user and putting it in @get_name array
    chomp(@get_name); #erasing newline from input
    #return @get_name;
}       

sub get_age
{
    print "Age :";
    push(@get_age, <STDIN>); #taking name from user and putting it in @get_name array
    chomp(@get_age); #erasing newline from input
    #return @get_age;
}       

sub get_gpa
{
    print "GPA: ";
    push(@get_gpa,<STDIN>);  #taking name from user and putting it in @get_name array
    chomp(@get_gpa); #erasing newline from input
    #return @get_gpa; #returning value may be unnecessary
}       

sub continue
{
    print "Get student data: ";
     $stu_data = <STDIN>;

    if ($stu_data =~ m/[n]/)
    {
            print "\nEnter file name: "; $user_file = <STDIN>;
            open (FILE, ">>$user_file") or die $!;
            printf FILE "%-0.25s %3.3s %4.4s\n", @get_name, @get_age, @get_gpa;           #writing to user given file.
            exit;

    }
    elsif ($stu_data =~ m/[y]/)
    {       #possibly makes do while loop unnecessary
            &get_name;
            &get_age;
            &get_gpa;
            &continue;
    }


}
close FILE;

Answer 1

Because of the way you're taking input, pushing onto a list, each push(@get_name, <STDIN>); will read in lines until the user hits Ctrl-D. This is a bad user interface, but it also means multiple lines can be pushed onto @get_name and friends. Each line is an element of an array. For demonstration...

$ perl -we '@lines = <STDIN>;  for my $line (@lines) { print "Line: $line"; }'
Foo
Bar
Line: Foo
Line: Bar

This will happen if the user hits enter twice expecting input to end.

What you want instead is my $name = <STDIN>; push @names, $name;. This will read in just one line to $name and then automatically stop. Then you push that single name onto @names.

Then on the output side there's another problem. printf takes a list of arguments. Perl doesn't know the difference between passing in three lists with one element and passing in a list with three elements. They are all flattened into one big list. So printf FILE "%-0.25s %3.3s %4.4s\n", @get_name, @get_age, @get_gpa just mashes all the names, ages and gpas into one big list and printf only prints the first three. So if there are two students, you're passing in name1, name2, age1, age2, gpa1, gpa2. You need a loop.

for my $idx (0..$#get_name) {
    printf FILE "%-0.25s %3.3s %4.4s\n", $get_name[$idx], $get_age[$idx], $get_gpa[$idx];
}

That says to loop from 0 to the highest index in @get_name. This is one minus the number of items in the loop. For historical reasons, arrays start counting at zero. Then you can feed printf the zeroth, first, second... entry in each array.

Going deeper, there are three ways to improve your code.

First is better use of subroutines. Instead of the routines setting global variables, they can return their results.

sub get_name
{
    printf("Name :");
    my $name = <STDIN>;
    chomp($name);
    return $name;
}

# @names is a better name because it describes what it contains, not how you got it
push @names, get_name;

Same for the rest. Once you've done that, it becomes clear you don't need three routines, but one. The only thing which changes is the prompt.

sub get_input {
    my($prompt) = @_;  # this pulls in subroutine arguments

    print("$prompt: ");  # no need to use printf here
    my $input = <STDIN>;
    chomp($input);

    return $input;
}

push @names, get_input("Name");
push @ages,  get_input("Age");
push @gpas,  get_input("GPA");

Now instead of maintaining three identical routines, and three opportunities for bugs, you have one.

continue is a poor choice for a subroutine name because it is a keyword in Perl. You'd be better calling this main as it is the main part of your program, a typical convention.

Rather than continue calling itself, something called recursion which is really useful but not necessary here, it's better to use a while loop.

sub main {
    while(1) {  # this is how you say "loop forever"
        print "Please enter data about a student.\n";
        push @names, get_input("Name");
        push @ages,  get_input("Age");
        push @gpas,  get_input("GPA");

        print "Would you like to enter more data?\n";
        my $more = <STDIN>;
        last if !$more =~ m/y/;  # last exits the current loop
    }

    print "\nEnter file name to output the data to: ";
    my $data_file = <STDIN>;
    open(my $fh, ">>$data_file") or die $!;

    for my $idx (0..$#names) {
        printf $fh "%-0.25s %3.3s %4.4s\n", $names[$idx], $ages[$idx], $gpas[$idx];
    }
}

Your printf has problems. In %3.3s the .3 part doesn't have any meaning for a string. For a number it means to print it to three decimal places. But you've used %s which means a string. I don't know what you're going for. With GPA I'm going to assume your intent was to print like 2.30. That's %.2f (f for "floating point" which is the confusing way programmers say "decimal number"). Age is always going to be an integer, so that's %3d. Yes, d for... integer.

If you want to read this data back in again and separate the fields, a usual way of doing this is to separate the fields with tabs. Then you can split up each line on tabs when reading it back in. Putting that all together...

    for my $idx (0..$#names) {
        printf $fh "%s\t%3d\t%.2f\n", $names[$idx], $ages[$idx], $gpas[$idx];
    }

The next step to explore would be to restructure your data. Having data about each student split between multiple arrays makes it awkward to coordinate and pass around into subroutines. Instead you'd use a hash.

my %student;
$student{name} = get_input("Name");
$student{age}  = get_input("Age");
$student{gpa}  = get_input("GPA");
push @students, \%student;

@students now contains a list of hash references. This is getting a little advanced, and I'm skimming a bit, but now you can get the name of the first student like so.

my $first_student = $students[0];
my $name = $first_student->{name};

And when printing them all out, the loop looks like this.

for my $student (@students) {
    printf $fh "%s\t%3d\t%.2f\n", $student->{name}, $student->{age}, $student->{gpa};
}

All up, it looks like this.

use strict;
use warnings;

main();

sub main {
    my @students;

    # Get students
    while(1) {  # this is how you say "loop forever"
        print "Please enter data about a student.\n";
        my %student;
        $student{name} = get_input("Name");
        $student{age}  = get_input("Age");
        $student{gpa}  = get_input("GPA");
        push @students, \%student;

        print "Would you like to enter more data?\n";
        my $more = <STDIN>;
        last unless $more =~ m/y/;  # last exits the current loop
    }

    # Get the file to output to
    print "\nEnter file name to output the data to: ";
    my $data_file = <STDIN>;
    open(my $fh, ">>$data_file") or die $!;

    # Dump the students into the file
    for my $student (@students) {
        printf $fh "%s\t%3d\t%.2f\n", $student->{name}, $student->{age}, $student->{gpa};
    }
}

sub get_input {
    my($prompt) = @_;  # this pulls in subroutine arguments

    print("$prompt: ");  # no need to use printf here
    my $input = <STDIN>;
    chomp($input);

    return $input;
}

Answer 2

This line in your code is printing 3 elements in total: (One printf with 3 %s-elements):

printf FILE "%-0.25s %3.3s %4.4s\n", @get_name, @get_age, @get_gpa;

This is not what you want to do. It will basically concatenate all arrays and print the first 3 elements. If all arrays have two elements you will get name1 name2 age1. If all arrays have 3 elemens name1 name2 name3. Etc.

Try it like this:

for my $i (0..$#get_name) {
    printf FILE "%-0.25s %3.3s %4.4s\n", $get_name[$i], $get_age[$i], $get_gpa[$i];
}

There are several ways to write this code in perl. This is the c-like version. Be careful: this loop assumes that all 3 arrays have the same number of elements.