[CatchThi5Drift]

I figured it out, but if anyone would still like to post a solution, that would be great. I'm sure mine (below) is pretty clumsy and convoluted in comparison. Thanks

Hello. I'm working on an assignment which involves opening a file, asking a user for input, and then writing their input into the opened file. I think I've got that down fairly decently, but I can't figure out how to allow the user to input more than once before closing the file.

The program is supposed to run until the user inputs "NOMORE" for the country name (meaning after they hit enter for the city, it should ask for another country name):

$test="FILEPATH/testing.txt";

if(-e $test) {
	print "File already exists.\n";
	print "Overwrite(1) exsiting file or append(2) existing file? (1/2): ";
	$choice=<STDIN>;

	if($choice=="2"){
		open TEST, ">>$test" or die "$test could not be opened for append: $!";
	} else{
		open TEST, ">$test" or die "Couldn't overwrite $test: $!";
	}
} else {
	open TEST, ">>$test" or die "$test could not be opened for writing: $!";
}

print "\nWhen you are done entering countries, type NOMORE.\n";

print "\nEnter a country: ";
$country=<STDIN>;
chomp ($country);

if ($country eq "NOMORE"){
	close (TEST);
} else {
	print "Enter the capital of $country: ";
	$capital=<STDIN>;
	chomp ($capital);
	
	print TEST "$country, $capital.\n";
}

open(TEST, "<$test") or die "Could not open $test: $!\n";

while(defined($a=<TEST>)) {
	print "\n$a";
}

close (TEST);

When I try to add a while loop, the program skips over everything I have in the loop:

print "\nWhen you are done entering countries, type NOMORE.\n";

while(<TEST>){
	print "\nEnter a country: ";
	$country=<STDIN>;
	chomp ($country);

	if ($country eq "NOMORE"){
		close (TEST);
	} else {
		print "Enter the capital of $country: ";
		$capital=<STDIN>;
		chomp ($capital);
	
		print TEST "$country, $capital.\n";
	}
}

open(TEST, "<$test") or die "Could not open $test: $!\n";

Any help or suggestions you can offer would be fantastic.

Thanks

What I came up with:

$test="FILEPATH/testing.txt"; # assigns file to $test

if(-e $test) { # check to see if file already exists
	print "File already exists.\n"; # if it does, tell the user
	print "Overwrite(1) exsiting file or append(2) existing file? (1/2): "; # ask user if they want to overwrite or append file
	$choice=<STDIN>; # user inputs choice

	if($choice=="2"){ # if user wants to append...
		open TEST, ">>$test" or die "$test could not be opened for append: $!"; # open the file to be appended
	} else{ # otherwise...
		open TEST, ">$test" or die "Couldn't overwrite $test: $!"; # open the file to be overwritten
	}
} else { # if the file does not exist...
	open TEST, ">>$test" or die "$test could not be opened for writing: $!"; # create it
}

print "\nWhen you are done enter NOMORE as country name.\n"; # give user instructions

print "\nEnter a country: "; # ask user for name of country
$country=<STDIN>; # assign user input to $country
chomp ($country); # remove newline from $country

if ($country eq "NOMORE") {
	close (TEST); # close the file if user enters NOMORE
}else{ # otherwise...
	print "Enter the capital of $country: "; # ask them for the capital of $country
	$capital=<STDIN>; # user inputs capital
	chomp ($capital); # remove newline from $capital
	
	print TEST "$country, $capital.\n"; # print the country and capital to the file
}

# the following will repeat until user inputs NO MORE
while ($country ne "NOMORE"){
	print "\nEnter a country: ";
	$country=<STDIN>;
	chomp ($country);

	if ($country eq "NOMORE") { 
		close (TEST); # close file if user enters NOMORE
	}else{
		print "Enter the capital of $country: ";
		$capital=<STDIN>;
		chomp ($capital);
		
		print TEST "$country, $capital.\n"; # print to file
	}
}

open(TEST, "<$test") or die "Could not open $test: $!\n"; # re-open file as read-only

# print contents of file
while(defined($a=<TEST>)) {
	print "\n$a";
}

close (TEST); # close file after contents have been listed

This post has been edited by CatchThi5Drift: 27 October 2009 - 07:22 PM

[dsherohman]

CatchThi5Drift, on 27 Oct, 2009 - 05:37 PM, said:

I figured it out, but if anyone would still like to post a solution, that would be great. I'm sure mine (below) is pretty clumsy and convoluted in comparison. Thanks

Sure, I'll give you a review of your posted solution.

First off, you should always start off your Perl code with

use strict;
use warnings;

"strict" will protect you against making a wide variety of common mistakes at the small cost of requiring you to explicitly declare your variables (usually with "my") the first time you use them. "warnings" will inform you of things which are less serious, but still probably not what you really wanted.

CatchThi5Drift, on 27 Oct, 2009 - 05:37 PM, said:

$test="FILEPATH/testing.txt"; # assigns file to $test

"$test" isn't really a very good variable name, since it doesn't hold a test. It holds a file name, so I'd recommend calling it "$file_name" instead. I'm also going to add a separate variable to hold the path, making this

my $file_path = '.';
my $file_name="$file_path/testing.txt"; # assigns file to $file_name

CatchThi5Drift, on 27 Oct, 2009 - 05:37 PM, said:

	if($choice=="2"){ # if user wants to append...
		open TEST, ">>$test" or die "$test could not be opened for append: $!"; # open the file to be appended
	} else{ # otherwise...
		open TEST, ">$test" or die "Couldn't overwrite $test: $!"; # open the file to be overwritten
	}
} else { # if the file does not exist...
	open TEST, ">>$test" or die "$test could not be opened for writing: $!"; # create it
}

The use of typeglob filehandles (i.e., bare ALLCAPS words) and two-argument open are a very old style. The better way of doing things today is to use lexically-scoped filehandles, which are non-global and basically just like regular (scalar) variables, and the three-argument open, which provides some protection against potential attacks on your program. And, again, TEST is a poor name, since it's the output file, not a test, so I'm replacing it with $output_file:

		if($choice=="2"){ # if user wants to append...
				open $output_file, '>>', $file_name or die "$file_name could not be opened for append: $!"; # open the file to be appended
		} else{ # otherwise...
				open $output_file, '>', $file_name or die "Couldn't overwrite $file_name: $!"; # open the file to be overwritten
		}
} else { # if the file does not exist...
		open $output_file, '>', $file_name or die "$file_name could not be opened for writing: $!"; # create it
}

CatchThi5Drift, on 27 Oct, 2009 - 05:37 PM, said:

print "\nEnter a country: "; # ask user for name of country
$country=<STDIN>; # assign user input to $country
chomp ($country); # remove newline from $country

if ($country eq "NOMORE") {
	close (TEST); # close the file if user enters NOMORE
}else{ # otherwise...
	print "Enter the capital of $country: "; # ask them for the capital of $country
	$capital=<STDIN>; # user inputs capital
	chomp ($capital); # remove newline from $capital
	
	print TEST "$country, $capital.\n"; # print the country and capital to the file
}

You don't need to do a standalone run through this before starting the loop, so I've removed it in my version.

CatchThi5Drift, on 27 Oct, 2009 - 05:37 PM, said:

# the following will repeat until user inputs NO MORE
while ($country ne "NOMORE"){
	print "\nEnter a country: ";
	$country=<STDIN>;
	chomp ($country);

	if ($country eq "NOMORE") { 
		close (TEST); # close file if user enters NOMORE
	}else{
		print "Enter the capital of $country: ";
		$capital=<STDIN>;
		chomp ($capital);
		
		print TEST "$country, $capital.\n"; # print to file
	}
}

While the loop does work this way, and I've left it alone in my cleaned-up version below (primarily to demonstrate that it does work without the pre-loop pass through getting country/capital), I would use "last" if I were writing this myself. My version of the code includes the loop with "last", but commented out, so that you can see how that way would be done.

CatchThi5Drift, on 27 Oct, 2009 - 05:37 PM, said:

open(TEST, "<$test") or die "Could not open $test: $!\n"; # re-open file as read-only

Three-argument open again:

open $output_file, '<', $file_name or die "Could not open $file_name: $!\n"; # re-open file as read-only

CatchThi5Drift, on 27 Oct, 2009 - 05:37 PM, said:

while(defined($a=<TEST>)) {
	print "\n$a";
}

I don't know... Maybe there's a reason for having the output double-spaced with a leading blank line, but I think the results look better with:

while(my $line=<$output_file>) {
		print $line;
}

or (if you're not afraid of implicit variables) just

print while (<$output_file>);

Note that you don't need "defined" here and, even aside from it being an uninformative name, you should also never create a variable named $a or $b in Perl because $a and $b are magical. (They're used by the "sort" keyword.)

So, with all that taken into account, here's how I'd clean your code up:

#!/usr/bin/perl 

use strict;
use warnings;

my $file_path = '.';
my $file_name="$file_path/testing.txt"; # assigns file to $file_name

my $output_file;
if(-e $file_name) { # check to see if file already exists
		print "File already exists.\n"; # if it does, tell the user
		print "Overwrite(1) exsiting file or append(2) existing file? (1/2): "; # ask user if they want to overwrite or append file
		my $choice=<STDIN>; # user inputs choice

		if($choice=="2"){ # if user wants to append...
				open $output_file, '>>', $file_name or die "$file_name could not be opened for append: $!"; # open the file to be appended
		} else{ # otherwise...
				open $output_file, '>', $file_name or die "Couldn't overwrite $file_name: $!"; # open the file to be overwritten
		}
} else { # if the file does not exist...
		open $output_file, '>', $file_name or die "$file_name could not be opened for writing: $!"; # create it
}

print "\nWhen you are done enter NOMORE as country name.\n"; # give user instructions

# the following will repeat until user inputs NO MORE
my $country = '';
while ($country ne "NOMORE"){
		print "\nEnter a country: ";
		$country=<STDIN>;
		chomp ($country);

		if ($country eq "NOMORE") {
				close ($output_file); # close file if user enters NOMORE
		}else{
				print "Enter the capital of $country: ";
				my $capital=<STDIN>;
				chomp ($capital);

				print $output_file "$country, $capital.\n"; # print to file
		}
}

# # Alternate version of the loop, using "last"
# while (1) {
#   print "\nEnter a country: ";
#   my $country=<STDIN>;
#   chomp ($country);
# 
#   last if ($country eq 'NOMORE');
# 
#   print "Enter the capital of $country: ";
#   my $capital=<STDIN>;
#   chomp ($capital);
# 
#   print $output_file "$country, $capital.\n";
# }
# close ($output_file);

open $output_file, '<', $file_name or die "Could not open $file_name: $!\n"; # re-open file as read-only

# print contents of file
while(my $line=<$output_file>) {
		print $line;
}

# # Alternate version of output loop
# print while (<$output_file>);

close ($output_file); # close file after contents have been listed

[CatchThi5Drift]

Hey, thanks for taking the time to look it over. This is all pretty foreign to me so I appreciate you correcting, and showing alternatives for, what I did.

Cheers.