Snowman11 has asked for the wisdom of the Perl Monks concerning the following question:

Sorry for posting alot on this one bit of code. Just havin trouble again making it on the add a user area. I want the user to enter in the fields then check to see if all the fields are filled in. Then if they are filled in then print out what they have and ask them if they are sure. But i seem to get cut off after i add values to all the values it just starts the while loop again...

i know i should use strict and warnings but i just want it to get it working.

any comments would be great.

#!/usr/bin/perl #This script creates the user #Created by Mark Mierau, June 17,2005 choice(); exit; ############################# #Create the PassWord Fuction# ############################# sub pass{ system "cls"; $flag = 0; while($flag == 0 or $pass eq "") {printf "\n\nEnter Password: "; $pass = <STDIN>; chomp($pass); printf "\n\nRe-enter Password: "; $repass = <STDIN>; chomp($repass); if ($pass eq $repass){ $flag = 1; print"\nPassword Confirmed.\n";} else{print"\nPasswords Do not match try again.\n";} } ######################################## # Create the Create,delete Menu Fuction# ######################################## sub choice{ system "clear"; print "Palliser Regional Schools\n"; print "iT for Dominica Database\n"; print "--------------------------\n\n"; print "1. Create a user\n"; print "2. Delete a user\n"; print "3. Change a Password\n"; print "4. Mass User Creation\n"; print "5. Exit\n"; #Get input print "Your choice: "; $input2 = <STDIN>; chomp($input2); #----------If user picks 1 for Choice Function--------------# if($input2 == int 1) { system "clear"; adduser(); }#end of if(); #----------If user picks 2 for Choice Function--------------# if($input2 == int 2) { system "clear"; deluser(); }#end of if(); #----------If user picks 3 for Choice Function--------------# #if($input2 == int 3) # { # system "clear"; # # #do this; # # }#end of if(); # #----------If user picks 4 for Choice Function--------------# #if($input2 == int 3) # { # system "clear"; # # #do this; # # }#end of if(); # # #----------If user picks 5 for Choice Function--------------# if($input2 == int 5) { system "clear"; print "Exiting...\n\n\n"; exit; }#end of if(); #----------If user picks 0 for Choice Function--------------# if($input2 ne NULL || $input2 ne " ") { system "clear"; print "\a"; &choice; }#end of if(); }#end of loop }#end of sub ################################################ ################ ADD THE USER ################## ################################################ my $number = ""; sub adduser{ #create the array my @data = ( { query => 'First Name',}, { query => 'Last Name', }, { query => 'School', }, { query => 'Phone', }, { query => 'Position', }, { query => 'Role', }, ); MAIN_LOOP: while (1) { #greeting system("cls"); print "iT For Dominica User Database:\n", "Enter the Number then enter the value.\n"; #number the amt of items in list # !!!! Much easier to read: for my $i (0 .. $#data) { print "\t", $i+1, ". ", $data[$i]{query}, ": ", $data[$i]{value} || "", "\n"; }#END OF FOR(); print "\t0. Commit\n"; # !!!! Added loop to simply error handling. foreach ('once') { # !!!! Added instructions on how to exit: print "Type a number to choose to change: (0 to commit)"; $number = ""; $number = <STDIN>; chomp($number); if ($number eq "" || $number eq "0") { goto BLOG; }#EO iF(); # !!!! Make sure it's a number! if ($number =~ /[^0-9]/ || $number > @data) { print "Bad input!"; redo; # Ask again. }#EO IF(); }#EO FOR(); --$number; print "\n\nPlease enter your ", $data[$number]{query}, ": "; chomp($data[$number]{value} = <STDIN>); }#END OF WHILE LOOP if(scalar @data{value} eq "" || null){ print "You have not entered all the values. Please try again"; &adduser;#goes back to asking for array members sleep 5; };#end of if{}; BLOG: # !!!! Cleaned up this loop a lot. foreach my $field (@data) { my $query = $field->{query}; my $value = $field->{value}; if (defined($field->{value}) && $field->{value} ne "") { print("$query: $value\n"); sleep 1; }#EO IF(); }#EO FOREACH print "Are you sure ? "; my $answer = <STDIN>; chomp($answer); $answer = uc($answer); if ($answer ne "YES"){ print "Aborting user creation. No user has been added to the s +ystem!\n"; sleep 5; last; } ## Add the user $finitial = substr($data[0]{value},0,1); $lastname = $data[1]{value}; $newuid = $finitial . $lastname; $newuid =~ s/\s+//g; # remove whitespace $newuid =~ s/\.//g; # remove periods (if any)t +he g removes whitespace globally $newuid = lc($newuid); print "the user id will be $newuid\n"; sleep 20; last; # }#END OF IF --$number; print "\n\nPlease enter your ", $data[$number]{query}, ": "; $data[$number]{value} = <STDIN>; chomp($data[$number]{value}); }# END OF SUB #------------------------------------------# # DELETE THE USER # #------------------------------------------# sub deluser{ my $deluid; my $answer; system("clear"); print "Enter the uid of the user you would like to delete:"; $deluid = <STDIN>; chomp($deluid); print "Are you sure you want to delete user $deluid (Yes/No) ? "; $answer = <STDIN>; chomp($answer); $answer = uc($answer); if ($answer ne "YES"){ print "Deletion Aborted, no users have been deleted.\n"; return; } if(-e "/home/$deluid"){ #First tar the users home directory in case we need to restor +e if ( -e "/home/olduserdata/$deluid.tar"){ print "There is already an archived folder with this uid\n +"; print "Check the folder /homeolduserdata\n\n"; sleep 5; return; } print "Deleting user.."; system("tar cvf /home/olduserdata/$deluid.tar /home/$deluid"); system("gzip /home/olduserdata/$deluid.tar"); system("userdel -r $deluid"); print "A copy of the users folder has been placed in /home/old +usersdata\n"; sleep 10; } else { print"ERROR---USER DOES NOT EXIST\n\n"; sleep 5; return; } }# END SUB

READMORE tags added by Arunbear

Replies are listed 'Best First'.
Re: Adding a Users data
by Joost (Canon) on Jun 27, 2005 at 22:19 UTC
    I know i should use strict and warnings but i just want it to get it working.
    <patronize seriousness="medium">
    Now you know why you should add strict and warnings IMMEDIATELY when you start coding. If you don't, it WILL take longer to fix your problems later.

    Also, that code is horribly indented. Try running it through perltidy first. Then replace the goto LABEL statements with something better, like subroutines. Didn't you know they're considered harmful? :-)

    </patronize>

    update: I just realised that this code is supposed to run with root privileges. 8-| Please take some time to fix this code to run with strict and warnings enabled. You wouldn't want to remove some unfortunate user now, would you?

Re: Adding a Users data
by Nkuvu (Priest) on Jun 27, 2005 at 22:33 UTC
    Short answer: use strict and warnings, even just "to get it working." Off the cuff I don't think that will illustrate the problem here (logical error, not syntactical), but it won't hurt.

    But your input is stored into $input2 which is checked against a number (you don't need the 'int 1' bit, it can be just 1). Then you have a conditional:
    if($input2 ne NULL || $input2 ne " ")
    At this point $input2 has not been reset by new user input. So if they choose anything, $input2 will most definitely be ne " ". Which means you recursively call your choice subroutine. (Hint: This really isn't what you want)

    So you need to clean things up a bit. I'd suggest something like the following pseudo-code:

    good_input = 0; while (not good_input) { display menu get user input if user input == 1 do stuff for choice one set good_input true elsif user input == 2 do stuff for choice two set good_input true # continue processing choices with elsif, # but the last bit should be the default # case: else display error message }

    This is simplistic in its approach, I'm sure other monks have a better idea or style. But it's fairly easy to understand, I think.

    Also, as Joost suggested, do some cleanup of the code with perltidy. On top of that, you could narrow down the scope of future questions -- find what it is that's going wrong, and write the shortest code snippet you can to illustrate the problem. You already know the basic area -- it's running into an infinite loop on the choice menu.

      Short lengthening of the short answer: using strict and warnings helps me get things working way faster and more reliably than not, 99 times out of 100.

      It's worth it. Really.

Re: Adding a Users data
by tcf03 (Deacon) on Jun 28, 2005 at 00:03 UTC
    Here is a piece piece of code for something that I have worked on recently - it is similar in scope and may give you some ideas.

    I have changed some things such as how it checks for valid parameters, but this is fairly functional. BTW. If you use strict; and use warnings; you should also use diagnostics; when writing code - I find that it helps me during the debugging stages...

    Ted
    --
    "That which we persist in doing becomes easier, not that the task itself has become easier, but that our ability to perform it has improved."
      --Ralph Waldo Emerson