Probably this line fails to find all the expected things from the Excel file referred to by $excelfileis.

my $oBook = $oExcel->Parse("$excelfileis");
[download]

Have you tried adding use strict to your code to see whether there is some scoping problem? To check whether the $oBook in the for loop is actually the one you expect (and not an unitialized package global).

Hope this helps.

Liz

1. use strict may reveal a nunmber of errors.
2. use Data::Dumper; and print Dumper $oBook; just before the for loop, will show what $oBook contains.
3. Manually debug with Perl -d. You can determine what is there, what can be done, and why.

The $self->{filename} makes me think you are using xls_conv as an object method but you are not setting $self anywhere in the sub. I'd expect to see a line like my $self = shift; at the top.

In any case, you have a prototype on your sub (which shouldn't be the case for an object method) but aren't using any parameters.

Try use strict and it will complain about any variables that you should be declaring with my in your subs but are instead using as (undeclared) globals.

In addition to the errors and debugging tips the others have pointed out, you should know that use is equivalent to BEGIN { require 'Module X' } which means that no matter where your use statement occurs, it will be executed during compilation of your module. It belongs therefore at the top of the file, and not inside of this subroutine.

Also, how do you mean to call these subroutines? Like MyPackage::MySub or MyPackage->MySub? If it's the latter then you'll need to fetch the package name from @_ and you should just drop the prototypes since they'll be ignored.

I frequently prefer to put the use for a module that is used only in one sub in the body of the sub. The only problem I could see with this approach is if anything is imported; in that case I would agree that it belongs at the top, since it affects the whole package.

There aren't any negative consequences codewise from doing it, even if you are importing stuff. For example, occasionally when debugging I'll throw use Data::Dumper; print Dumper $foo in the middle of some code. Dumper is an imported subroutine in this usage and works just fine so long as it's used in the same package it's imported into.

The problem is not with writing functional code but with writing readable, maintainable code. Novice programmers, probably like the AnonyMonk here, might read such code and think mistakenly that use works at runtime like require or eval "use Foo";. Moreover, someone maintaining your code, even if an experienced Perl programmer, isn't generally going to scan your codebase looking for embedded use statements because it's customary to place them at the top of the file, and that could lead to confusion, for example in case of conflicts with imported symbols. In short, yeah you can get away with it, but it introduces a maintenance risk.

---

"The dead do not recognize context" -- Kai, Lexx

    for(my $iSheet=0; $iSheet < $oBook->{SheetCount}; $iSheet++)
    {
        print "$oBook->{SheetCount}";
    }
[download]

use strict;
use warnings;
use Benchmark qw/ timethese cmpthese /;

my $oBook = { SheetCount => 1000 };

cmpthese(timethese(10000, {
        'For_C'   => '&For_C',
        'For_Set' => '&For_Set',
    }));

sub For_C
{
    my $var;
    for (my $iSheet=0;
         $iSheet < $oBook->{SheetCount};
         $iSheet++)
    {
        $var = $iSheet;
    }
}

sub For_Set
{
    my $var;
    for my $iSheet (0..$oBook->{SheetCount}-1)
    {
        $var = $iSheet;
    }
}
[download]

Benchmark: timing 10000 iterations of For_C, For_Set...
     For_C: 14 wallclock secs (14.33 CPU) @ 697.84/s (n=10000)
   For_Set:  7 wallclock secs (7.22 CPU) @ 1385.04/s (n=10000)
          Rate   For_C For_Set
For_C    698/s      --    -50%
For_Set 1385/s     98%      --
[download]

$iSheet < $oBook->{SheetCount} # hash look up and comparison
[download]