To me the amount of comments in the code serve to make it less readable. If you need that many comments to allow people to understand your code perhaps you should rewrite the code to be more understandable. Use comments to explain why you did something and let the code should show how it's done. You write my %HoHoHoA; #used in readListOfFilesToSFTP() What does that serve to tell someone reading your code? I'm not sure what exactly but I am sure Santa Claus is somehow involved.

You've defined two constants TRUE and FALSE which aren't used anywhere in the code that I see. Check this page for an explanation of why that's usually a silly thing to do.

There are several blocks of code with a pattern of

do_this() if (! $interactive);
and_this() if (! $interactive);
and_this_too() if(! $interactive);
[download]

why not:

if (!$interactive) {
    do_this();
    and_this();
    and_this_too();
}
[download]

If looks like you are trying to see if put succeeds but in a really obscure way..

if (
    eval {
        $sftp->put( $file, $target_file, \&callback );
    } ne 0 
)
[download]

Does that work? From looking at the source, put returns undef on failure:

if ($sftp->put()) {
    # success
}
else {
    # fail
}
[download]

There are several areas where you are doing things like this:

  writeLog("* * * ABNORMAL COMPLETION * * * Error: $!. Unable to open 
+file $files_to_sftp" );
  print( STDOUT "* * * ABNORMAL COMPLETION * * * Error: $!. Unable to 
+open file $files_to_sftp\n" );
[download]

Why don't you make the writeLog function a little smarter rather than littering the code with all those checks for  if (!debug) and if (! $interactive) maybe set up a closure so that the writeLog function knows what mode it is in and knows what to do with the messages it receives something like:

sub set_up_logger {
    my ($interactive, $batch, $whatever) = @_;
    sub writeLog {
        my $msg = shift;
 
        if ($interactive) {
            print STDOUT $msg;
        }
        elsif ($batch) {
            print FILE $msg;
        }
        ## whatever whatever
    }
}
[download]

I see some system calls where the return value isn't being checked, bad idea.

Anyway those are just some quick comments I had while reading down through your code, I didn't actually run it or check too hard for bugs. I think overall it could benefit from more design work upfront to make it more modular. Best of luck!

---

---

• 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.