in reply to Trouble with an array inside a format call

I think jethro's comments are on traget with your error. I'll make a couple of other suggestions. Not trying to be hyper critical, just helpful.

1. When debugging stuff where you are getting run time messages to STDERR and printing stuff to STDOUT at the same time, often it is helpful to turn buffering off by putting $|=1; at the top of your code. STDERR is not buffered while STDOUT is and thus the order of the stuff on the screen won't reflect the actual time sequence of events unless you turn the buffering off.

2. I believe that your command loop could be improved. You've set main up as a subroutine that you call when you want to get started again (happens in a bunch of places in the code). This approach has a problem, every time you call main(), you leave some stuff on the stack. To illustrate the issue, make these 2 small changes to your code:

} elsif ( $option == 8 ) { return(0); ######change here } else { print "Error: Option $option invalid\n"; <STDIN>; main(); print "leftover\n"; ###change here <STDIN>; }
Now run the program and select an invalid option a couple of times, then select 8, the exit. You will see that "leftover" gets printed a few times these are the exits from main() that never happened before. Now in this program, I doubt that this would become any big deal. But this probably would get to be a problem in a long lived program that called main() a lot.

3. There are many ways to Rome on command loops. I wrote some quick code for you to show another way to do this - many variations are possible. A general philosophy of mine when dealing with user input: validate the heck out of it before trying to use it and be flexible in allowing users to enter whitespace around input.

#!/usr/bin/perl -w use strict; while (display_menu(), (my $option = <STDIN>) ) { $option =~ s/^\s*//; #no leading whitespace $option =~ s/\s*$//; #no trailing whitespace #(no need for chomp) as \r\n\t\f #and space all count as \s chars if ( ($option !~ /^\d$/) or ($option <=0) or ($option >2) ) { display_menu_error ("ERROR: illegal option: $option selected\n" +); next; } print "opt number is $option\n"; #just a debug statement action1() if $option == 1; action2() if $option == 2; #etc..... #note: this compare is so fast, that no need to clutter things #up with elseif statements } sub action1 { clear_screen(); print "OPERATION 1 ok: \n"; #could be an error message } sub action2 { clear_screen(); print "did action 2\n"; } sub clear_screen { if ( $^O =~ /MSWin32/ ) { system('cls'); } else { system('clear'); } } sub display_menu_error { my $msg = shift; clear_screen(); print "$msg\n"; } sub display_menu { print "Cleveland State CC Computer Repairs\n\n"; print "Version xxxxxxx\n", "Copyleft 2009 Adam Jimerson & Andy Arp\n"; print "----------------------------\n", "Main Menu, To make a selection specify the number for that action +\n"; print "1. Add New Computer to Database\n"; print "2. Edit Computer Status in Database\n"; print "3. Remove a Computer from Database\n"; print "4. Look up Computer Information\n"; print "5. List All Repair Requests\n"; print "6. Print Final Report For Customer\n"; #this should be don +e to ensure we provide information on a repair #to the customer on paper print "7. Check Computer Status\n"; print "8. Quit the program\n"; print "Action: "; }
Each action does what it does and then returns to the main command loop. Then program just falls through any $option compare statements and goes back to reprompt. So when one of these "action" subroutines finishes, either with some error message or success message, just return(), don't call main() again!

I tried to incorporate your idea of clearing the screen after each action. Although I remember answering your question about how to do this in XP, I personally would find that interface annoying and wouldn't do it that way. But this is your app, not mine! Go for it!

Anyway, main shouldn't be a subroutine.

3. I would move the database connection and authentication stuff to the top of program. Looks like every action depends upon DB. No need to do this multiple times and no need to bother user entering in a whole bunch of stuff only to find out later that operation is going to fail because you can't connect to the database.

4. I don't know how your experience will work out with FORMAT, but when I first saw this, I thought how cool! I wrote one program with it. Then decided that it was more hassle than other ways. I find that printf, sprintf do a better job for me. Your mileage may vary.

Replies are listed 'Best First'.
Re^2: Trouble with an array inside a format call
by chromatic (Archbishop) on Oct 15, 2009 at 21:10 UTC
    STDERR is not buffered while STDOUT is and thus the order of the stuff on the screen won't reflect the actual time sequence of events unless you turn the buffering off.

    I think this is misleading advice. STDOUT is line buffered. I skimmed all of the statements which print to STDOUT, and I don't believe your advice will have any effect.

      I certainly can't rule out some kind of OS or installation specific thing that makes your setup different than mine. I am using WinXP and ActiveState 5.10, although I know from experience this is the same on ActiveState 5.6.

      Here is a simple test program. Try it on your machine and see if you get the same results.

      #!/usr/bin/perl -w use strict; $|=1; #test was run with and without this statement #this turns buffering off print "this is a test\n"; print "this is a test2\n"; print STDERR "this is an error after line2\n"; print "this is a test3\n"; print "this is a test4\n"; __END__ Prints: without $|=1........ this is an error after line2 this is a test this is a test2 this is a test3 this is a test4 Prints: Now with $|=1 ...... this is a test this is a test2 this is an error after line2 this is a test3 this is a test4
      The STDERR message gets printed first because the lines from STDOUT haven't gone to the screen yet.

      Using $|=1 decreases I/O performance by maybe 30% or something like that. Normally of no consequence unless you are doing a lot of output or are trying to get the time sequence of things right.

      Update: deleted few lines about my version of tee.pl as there is a similar topic going on in another thread and I got them confused. Re^3: Getting user input with STDOUT tee to file.

        I certainly can't rule out some kind of OS or installation specific thing that makes your setup different than mine.

        I don't know a thing about Windows I/O, but the program behaves the same way (as I expect) on Linux with and without $| = 1;.

Re^2: Trouble with an array inside a format call
by vendion (Scribe) on Oct 15, 2009 at 18:43 UTC

    Thanks for the help first of all, also Marshall I like the way you have done it on your third suggestion and I was wondering if with it like that would #2 still be a problem?
    As for 4 format does its job and I can't argue with that, but my goal is to that the program is to have the program write format and its output to the /tmp/repairs file.

      I see that I inadvertently used #3 twice but, Yes, re-doing the command loop will solve the problem in #2.

      If you mean the DB authentication, I would authenticate early and this "my $dbh" (database handle) will have package scope, a my $var at the top package level which cannot be exported to other modules (because it is a "my"), but yet would be visible to any sub in the package.

      Even that you don't need to pass this to subs, I would pass the $dbh as a param to each "action" sub that needs it. The reason for that is to make it clear what each sub is using. You should wind up with a very small number of package level "real" variables. To further that I would suggest using the "constant" pragma.

      use constant database => "Repairs"; #the database name use constant hostname => "192.168.1.111"; #MySQL server IP address use constant port => '3306'; #MySQL port
      A constant is not a $variable and cannot be modified. You use it like a bareword (without any $ in front of it) in the actual code. I recommend this for package level things that are the "seldom changed, but important constants that might need to change in a different version of the program".

      You should wind up with a very small number of package scoped "real variables" that the program changes during execution.

      As far as Perl FORMAT goes, you will have to make up your mind about that as you write more code. It appears to work fine for strings, but when you want say exactly 2 decimal places for a currency number, this is not so good.

      BTW, for this heading thing, there is no need for a FORMAT statement. A simple way is to use what is called a "here is" document.

      #!/usr/bin/perl -w use strict; my $STDOUT_TOP= <<END_TOP; ID First Name Last Name Email Address Phone Number Dat +e Comp. Manufacturer Comp. Model Model Number OS ========== ========== ============ ================ ============ === + ======= ================== =========== ============ ============= END_TOP print $STDOUT_TOP;

        My goal with this is get it so It will have formatted output that is set to a temporary file for printing. When it is done and the page is printed I want the page to look like what my current format is, where there is a header saying what each field is and then the customer/computer info and everything lines up when it is done.

        If this is something that is better and easier done with printf/sprintf then that is fine with me.

Re^2: Trouble with an array inside a format call
by Anonymous Monk on Oct 18, 2009 at 12:19 UTC