in reply to RFC: Code testers/reviewers needed

Some comments on the code:

  1. Digest::MD5::md5_hex is imported but not used.

  2. $location is initialised via user input but then not used.

  3. Line 61 could be simplified to: my $ext = $1;

  4. Lines 71 and 72 could be written as a single line: $filename =~ s{.*/(.*)$}{$1};

  5. The next statements on lines 76 and 94 are redundant, as each occurs immediately before the loop iterates anyway.

  6. In addition, on line 94 the or clause does nothing. next can’t “fail”, and it doesn’t return a value.

  7. Lines 101 to 105 could be rewritten as a single line: print $error eq '' ? "No errors.\n" : "\nErrors: $error";

Hope that helps,

Athanasius <°(((>< contra mundum Iustus alius egestas vitae, eros Piratica,

Replies are listed 'Best First'.
Re^2: RFC: Script testers needed
by AI Cowboy (Beadle) on Jun 06, 2013 at 17:13 UTC

    In order:

    1. You're right! Digest::MD5::md5_hex was used previously to check the md5 hash of the file. I will remove it now that it is no longer used.
    2. Whoa, that's a bit of a big error on my part. I guess I edited the use of $location out, but that's not how it's supposed to be. Thanks for catching that.
    3. Good point. I'm not very well versed on the intricacies of regular expressions, thanks for the tip.
    4. Alright, thanks for the help there, once again regular expressions aren't something I'm used to using much of; usually just simple substitution or word-matching regex's.
    5. Good catch, they will be removed.
    6. Kind of a newby error to make, on my part. I was trying to simply make sure that many of the possible errors were found and reported to the user, and I didn't take the time to figure out if next even CAN have an error. I'll remove the or statement.
    7. Awesome, thanks for that tip :)

    That was a lot of help, I'll be sure to tweak the code up using your help! Thanks!

[reply]
[d/l]
[select]

In Section Meditations