Hello everyone, I'm new in perl and I'm a student in IT. I need to write some scripts for my internship. Over 3 weeks there is a jury who judge over the internship and they will see my scripts. But it could be that some of the people are experts in perl or scripting. I would like to ask if some of you will check my script and give some suggestions and feedback. The script is to make a dump of all the databases from remote. The scripts works fine normally. Many thanks for the suggestions!

Code

#!/usr/bin/perl # (c) Odeurs Dieter # Script to back-up the databases

use strict; use warnings; use Getopt::Long; use Net::Ping; use Time::Local;

Declaring all your lexical variables at the top of the program largely defeats the advantage of "use strict". All lexical variables should be declared in the smallest possible scope. Your use of them as globals in subroutines makes it even worse. It is far better to supply all inputs to a subroutine as arguments.

Your usage section should be a "here document" and a single print.

I would prefer an all perl solution rather than an external program to access the data base. I believe that the DBI module could do the job.

You should consider formatting the time stamp with the function strftime in POSIX.

You have several logic errors. The values undef, numeric zero, and null string are all false. Any other value is true.

Code

if ($? !=0) {

The "!=0" is unnecessary.

Code

if ($password && $password ne ''){

This is even worse. if $password is a null string, it is false and the second test is not performed. On the other hand if it is not, the second test is performed and always passes. I suspect that you mean:

Code

if (defined $password && $password ne ''){

Note: There are several other analagous errors.

Be prepared to defend your style of indenting and nesting. It is not my first choice, but all that really matters is that you are consistent. You are. Good Luck, Bill