Re: Phone Book

Break up your Choice logic so that it calls subroutines that do the actual work. It will make your code easier to understand if you do this,

$userInput = <STDIN>;
chomp $userInput;
$userInput = uc $userInput; 
# you could probably do this on the STDIN, but I am not sure.
# Another monk should be able to tell you.

    ( $userInput eq 'L' or $userInput eq 'LIST'  ) ? list_phone_book()
  : ( $userInput eq 'A' or $userInput eq 'ADD'   ) ? add_to_phone_book
+()
  : ( $userInput eq 'D' or $userInput eq 'DELETE') ? delete_from_phone
+_book()
  : ( $userInput eq 'Q' or $userInput eq 'QUIT'  ) ? quit_phone_book()
  :                                                  unknown_selection
+();
[download]

As it stands now, you would have read your whole script line by line to figure out where you are going for your selections. With the above code, you know where to look for each selection and what it is supposed to do.

You are also doing a bunch of open and closes all over the place, I beat your could get rid of a bunch of code by breaking those down in subroutines.

Comment on Re: Phone Book Download Code

Replies are listed 'Best First'.
Re^2: Phone Book by Lawliet (Curate) on Jan 04, 2009 at 20:02 UTC
`$userInput = <STDIN>; chomp $userInput; $userInput = uc $userInput;` [download] can be written (more concisely) as `chomp(my $userInput = uc <STDIN>);` Also, I think there may be a better method to running each subroutine than using multiple conditional operators. Perhaps a dispatch table will do. `my %pbactions = ( L => \&list_phone_book, LIST => \&list_phone_book, A => \&add_phone_book, ... ); # Later on~ defined $pbactions{$userInput} ? $pbactions{$userInput}->() : usage();` [download] Untested lines of code, they're just to inspire. And you didn't even know bears could type.	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re^2: Phone Book
by Lawliet (Curate) on Jan 04, 2009 at 20:02 UTC

$userInput = <STDIN>;
chomp $userInput;
$userInput = uc $userInput;
[download]

can be written (more concisely) as

chomp(my $userInput = uc <STDIN>);

Also, I think there may be a better method to running each subroutine than using multiple conditional operators. Perhaps a dispatch table will do.

my %pbactions = (
    L    => \&list_phone_book,
    LIST => \&list_phone_book,
    A    => \&add_phone_book,
    ...
);

# Later on~

defined $pbactions{$userInput} ? $pbactions{$userInput}->()
                               : usage();
[download]

Untested lines of code, they're just to inspire.

And you didn't even know bears could type.

[reply]
[d/l]
[select]