comment on

I never considered myself a talented coder/programmer/scipter but I know enough to build some useful tools that don't break.....often ;)

As with many shops we have scripts that you "don't touch because they work." One in particular is our Unix account creater that was written by a "Perl Guru" who was paid a great deal of money to automate several funtions.

After taking a class, reading a few books, reading stuff from the Net, and writing a few reports. I was asked to add a "feature" to the account creater.

I reviewd the source and I now want to be a consultant. The guy who wrote this "automation" took a rather heavy handed approach.

The following is a piece of the mess. Basically, you are prompted for a username, the passwd file gets loaded to an array, it checks for duplicates, and it looks for an open userid:

system("clear");
print "CREATE ACCOUNT:\n\n";
$username = &ask("Enter the login id of the new user: ");

#
# Read the local passwd file.
#
open (PWD,"$LOCAL_PASSWD")|| die "can't open $LOCAL_PASSWD\n";
while (<PWD>) {
   push(@local_passwd,$_);
}
close (PWD);

#
# Figure out what uids have been used
#
for ($i=0 ; $i<=$#local_passwd && ($_ = $local_passwd[$i]); $i++) {
   ($uname,$pwd,$uid) = split(/:/);

   push(@user_ids,$uid);
}

foreach $i (200..2000) {
   foreach $uid (@user_ids) {
      $uid_ok=1;
      if ($i eq $uid) {
         $uid_ok=0;
         last;
      }
   }

   if ($uid_ok) {
      $g_user_id = $i;
      last;
   }
}


undef @user_ids;

#
# Read the local password
#
open(PWD,"$LOCAL_PASSWD")|| die "can't open the $LOCAL_PASSWD\n";
while (<PWD>) {
   push(@local_passwd,$_);
}
close (PWD);

#
# Check for duplicate names
#
for ($i=0 ; $i<=$#local_passwd && ($_ = $local_passwd[$i]); $i++) {
   ($uname) = split(/:/);

   if ($uname eq $username) {
      print "DUPLICATE NAME: $username: can't continue\n";
      exit(1);
   }
}
undef @local_passwd;
[download]

Like I said, it does the job but in a rather heavy handed way. Guess the guy never heard of hashing.

I suggested a re-write and of course the bosses heart rate went up. So I just got the above example and threw together some code to show a difference.

print $clear_screen;
print "CREATE ACCOUNT:\n\n";
$username = &ask("Enter the login id of the new user: ");
my %account;
my %uid;
#
# Read the local passwd file.
#
open (PWD,"$LOCAL_PASSWD") or die "\n\nUnable to open $LOCAL_PASSWD\n\
+n";
while (<PWD>) {
   chomp;
   my ($accountname, $accountid) = (split/:/)[0,2];
   $account{$accountname} = $accountid;
   $uid{$accountid} = $accountname;
}
close (PWD);

if (exists $account{$username}) {
   die "This account name already exists!\n";
}

#
# Loop through our range and see if we have an available number.
# If a hash entry does not exist, use the missing number.
foreach (200..2000) {
   unless ( exists $uid{$_} ) {
      print "Hey $_ is available!...assigning...\n";
      $g_user_id = $_;
      last;
   }
}
[download]

After seeing the reduction, he ok'd the rewrite.

Powerful computers can be a friend as well as an enemy. Sure the script does the job and it isn't even slow, but was the approach right? I don't think so. In this age of "put it in now, we will fix it later" bad code seems to slip in more and more. I should ask for a bonus for rewriting the "experts" scripts. ;)

All in all, I guess this is an example of how you should have people that understand the language and programming in general to interview consultants.

Edited: 04 July 2002, by footpad
Reason: Added <readmore>, per Consideration.

In reply to So you know Perl; but do you know programming? by Marza

Are you posting in the right place? Check out Where do I post X? to know for sure.
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
Want more info? How to link or How to display code and escape characters are good places to start.


Keep It Simple, Stupid
	PerlMonks