For cheap thrills, I reviewed your script by going through
the principles in On Coding Standards and Code Reviews and making a few notes every time I pulled a face:
- Separate user versus maintainer documentation. For a simple script like yours, I'd put the user doco in a usage subroutine (just a big here-doc with an exit 1 at the end) and call this sub if the user enters -h or --help or an invalid command line; suggest the standard Getopt::Long to parse command line arguments ... which should make your commands easy to learn and use because they behave just like system commands. I suggest using simple Perl comments (not POD) for the maintainer documentation.
- Handle all errors (e.g. don't ignore error returns). Fail securely. Apart from not handling command line errors (mentioned in the previous point), you are not checking system and opendir for errors (BTW, you should use a lexical variable (not DIR) when calling opendir). For now, just add an or die to the opendir while giving some thought to the next point.
- Establish a rational error handling policy and follow it strictly. Just FYI for now. For your script, failing by writing a clear error message to stderr and returning non-zero exit code seems appropriate.
- Any unexpected result from a file operation or API call or external command should be logged. This sort of thing is invaluable when supporting a product on customer sites - probably not appropriate for your scripts at the moment.
- Include a comment block on every non-trivial method describing its purpose. Just one or two comment lines describing each of your functions seems appropriate.
- Minimize the scope of variables, pragmas, etc. The six variables used for the command line arguments are all global. Do they need to be? Keeping state in global variables is a nightmare in large programs. You might like to think about how to reduce the number of global variables.
- Short names for short scopes, longer names for longer scopes. You are already doing this quite well in that your six global variables all have long descriptive names.
- Design interfaces that are: consistent; easy to use correctly; hard to use incorrectly; easy to read, maintain and extend; clearly documented; appropriate to your audience. You are already doing this quite well - using the standard command line parsing mentioned in the first point will further improve.
- Write the test cases before the code. You can probably get away with manual testing for now. To improve your skills for the future, think about how to test your code, especially how you might automate testing (see, for example, Test::More and Effective Automated Testing).
You are already off to a great start with your first script. Well done!
