#7 Code cleanup

closed
nobody
None
5
2014-08-14
2004-05-25
No

Just a few suggestions... I am not out to offend but to
help. ;)

1. If you don't like having people edit the file and
would perfer a ./configure install solution, see
pconfig @ http://freshmeat.net/projects/pconfig/

2. The 'my ($pidcmd)' syntax is not needed. When you
have the ()s, that tells perl you are setting an ARRAY
not a SCALAR. So basically, it wastes time because perl
takes the right hand side, says to itself "but, I need
an array here", converts it to an array, and then sets
$pidcmd to the first element of the array it created,
which is exactly what you wanted in the first place.

3. I accidentally included in my patch the note to look
into Getopt::Long for a 'standard' way of doing
switches. Also a different approach to do what you are
doing - you can name anything, not just the loop
defined as SWITCH.

4. "chomp" is much safer than "chop" assuming you only
want to remove newlines.

5. I hate code redundancy - it is an excuse for a bug
to crop up. Right now, there is a large chunk of
display code in two places - if there is color or not.
If you fix a bug (or add a feature like I did) to one
and not the other, it gets messy. I recommend doing
something like this:

if ($color) {
$start = '\e[01;31m';
$end = '\e[00m';
} else {
$start = '';
$end = '';
}

And then you only have one print statement for both
color and no color:
defined($to{$msg}) and print " ${start}To${end}:
$to{$msg}\n";

- Aaron

Discussion

  • Christopher Chan

    • status: open --> closed
     
  • Christopher Chan

    Logged In: YES
    user_id=1391925
    Originator: NO

    closed due to massive changes in latest qmhandle

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks