Re: Thanks !! (XML + Excel) with (win32::OLE and XML::Simple)

in reply to Thanks !! (XML + Excel) with (win32::OLE and XML::Simple)

Here're my code review comments:

The opendir/readdir/closedir lines become one using glob or something like File::Find::Rule.
$dir is set, but then "." is hardcoded in the copendir.
Prompt text says "all the xml files in the scripts folder" but the script looks for ./*.xml
```
my $a = "x";            
while ($a ne "y")
{
    ...
    chop $a;
}
[download]
```
can be: while( my $input ne "y" ){ ... ; chomp $input; }
Avoid $a and $b as var names, since they are non-descriptive, and have special meaning (see sort).
Don't blindly chop -- use chomp.
Or, just remove that whole while block ... if they didn't wany it run, they shouldn't have executed the script :)
(yes, you could argue that they ran the script accidentally. Well, then i'd say it's just as likely they accidently hit "y" when they meant "n" to the confirmation, and you should ask again. and again. and .... :) ) Just makes the script have less moving parts (reaD: easier to maintain).

unless ($files[0]){
    print "\nSorry, no xml file(s) in script folder\n\nScript Terminat
+ed...\n\n";
    }
    else{
    ...  # a gazillion rows
}
[download]

This should read (use array in scalar context; and avoid large block):

unless( scalar @files ){
  warn "no files";
  exit;
}
...
[download]

You may even want to die instead.

You might want to make a parse_file() sub, so the main code then becomes:
```
parse_file($_) for @files;
[download]
```
for my $i(0 .. $count) {
This is most commonly written as
```
foreach my $i ( 0 .. $count ){

<li> to be more explicit (since you want an array), do <c>unless( ref(
+$doc->{$sub1}) eq 'ARRAY' ){
[download]
```
Does it even happen that these aren't arrays? if so, nod. if not, maybe just ditch that clause (and if it ever happens by mistake, it'll just error out).

I think the nested loops become easier to deal with if you work with array values instead of indexes ... something like:

foreach my $sub1 ( sort keys %{ $doc } ){

  ... # $sub1

    foreach my $sub2Hash ( @{ $doc->{$sub1} } ){

        for my $sub2 ( sort keys %$sub2Hash ){

            unless (ref($sub2Hash{$sub2})) {

                ...  # $sub2
                $worksheet->Cells($rowCount, $colCount)->{Value} = $su
+b2;
                $worksheet->Cells($rowCount, $colCount)->{Value} = $su
+b2Hash{$sub2};
                next;

            }

            foreach my $sub3Hash( @{$sub2Hash{$sub2}} ){

                for my $sub3 ( sort keys %$sub3Hash ){
[download]

BUT, I notice that, at each nested level, it all looks the same -- should break that out into a function. Here's something that does that, and the previous comment about using variables as you go:

sub __add_non_array {
  my ($worksheet, $rowCount, $colCount, $sub) = @_;
  $worksheet->Cells($rowCount, $colCount)->{Value} = $sub;
  $worksheet->Cells($rowCount, $colCount)->Borders() -> {Weight} = 2;
  $colCount++;
  $worksheet->Cells($rowCount, $colCount)->{Value} = $doc->{$sub};
  $worksheet->Cells($rowCount, $colCount)->Borders() -> {Weight} = 2;
  $rowCount++;
  $colCount--;
  return ($rowCount, $colCount);
}

while( my ($sub1, $aoh1) = each %$doc ){

    unless (ref($aoh1) eq 'ARRAY') {
      ($rowCount,$colCount) = __add_non_array($worksheet, $rowCount, $
+colCount, $sub1);
      next;
    }

    foreach my $sub2Hash ( @$aoh1 ){

       while( my ($sub2, $aoh2) = each %$sub2Hash ){

          unless (ref($aoh2) eq 'ARRAY') {
            ($rowCount,$colCount) = __add_non_array($worksheet, $rowCo
+unt, $colCount, $sub2);
            next;
          }

            foreach my $sub3Hash ( @$aoh2 ){

                while( my ($sub3, $aoh3) = each %$sub3Hash ){

                  unless (ref($aoh3) eq 'ARRAY') {
                    ($rowCount,$colCount) = __add_non_array($worksheet
+, $rowCount, $colCount, $sub3);
                    next;
                  }

                    foreach my $sub4Hash ( @$aoh3 ){
                        print "Collecting general information...\n";

                        while( my ($sub4, $aoh4) = each %$sub4Hash ){

                          unless (ref($aoh4) eq 'ARRAY') {
                            ($rowCount,$colCount) = __add_non_array($w
+orksheet, $rowCount, $colCount, $sub4);
                            next;
                          }

                            print "Collecting $sub4 ...\n";

                            $subDataRow = scalar(@$aoh4) + ($rowCount 
++ 1);

                            my $worksheet = $workbook->Worksheets->Add
+({After=>$workbook->Worksheets($workbook->Worksheets->{Count})});
                            $worksheet -> {Name} = $sub4;
                            $worksheet -> Range("A1") -> Font -> {Size
+}= 14;
                            $worksheet -> Range("A1") -> Font -> {Colo
+rIndex}= 2;
                            $worksheet -> Range("A1") -> {Value} = "$s
+ub4";
                            $worksheet -> Range("A1:E1") -> Merge;
                            $worksheet -> Range("A1:E1") -> Interior -
+> {ColorIndex} = 25;
                            $worksheet -> Range("A1:E1") -> Borders() 
+-> {Weight} = 3;
                            my $subDataCol = 1;

                            foreach my $sub5Hash ( @$aoh4 ){

                                my $temp = "";

                                while( my ($sub5, $aoh5) = each %$sub5
+Hash ){
                                    my $curID = $sub5Hash{enclosureNum
+ber} || $sub5Hash{number} || 0;;
                                    my $nextup = "";

                                                     if(ref($aoh5) eq 
+'ARRAY'){
                                                     #################
                                                     # SUBINFORMATIE #
                                                     #################
+  $sub 4 is rank $sub5 is attribuut van rank $sub6 is element binnen 
+rank eg arrayref

                                                         foreach my $s
+ub6Hash ( @$aoh5 ){

                                                                 # Nie
+uwe ArayRef, ExtentbyVol,StorageDeviceFRY (nieuwe titels maken

                                                                 if($n
+extup ne $sub5){
                                                                     $
+new = 1;

                                                                     $
+subDataRow++;
                                                                     $
+subHeaderRow = $subDataRow;
                                                                     $
+subDataCol=1 ;
                                                                     $
+nextup = $sub5;
                                                                 }
                                                                 else 
+{$new = 0;}

                                                $subDataRow++;

                                                              while( m
+y ($sub6, $val6) = each %$sub6Hash ){

                                                        # subinfo titl
+e on new element
                                                        if ($temp ne $
+curID){

                                                            $temp = $c
+urID;

                                                            $worksheet
+->Cells($subDataRow, $subDataCol)->{Value} = "$sub4 : $curID";
                                                            $worksheet
+->Cells($subDataRow, $subDataCol)->Interior->{ColorIndex} = 33;
                                                            $worksheet
+->Cells($subDataRow, $subDataCol)->{Font}->{Bold} = 1;
                                                            $worksheet
+ ->Cells($subDataRow, $subDataCol)-> Borders() -> {Weight} = 1;
                                                            $subDataRo
+w+=2;
                                                            $subHeader
+Row = $subDataRow;
                                                            $subDataRo
+w++;
                                                        }

                                                        # Header
                                                         if($new){
                                                            $worksheet
+->Cells($subHeaderRow, $subDataCol)->{Value} = $sub6;
                                                            $worksheet
+->Cells($subHeaderRow, $subDataCol)->{Font}->{Bold} = 1;
                                                            $worksheet
+ ->Cells($subHeaderRow, $subDataCol)-> Borders() -> {Weight} = 2;
                                                            $worksheet
+ ->Cells($subHeaderRow, $subDataCol)-> Borders() -> {ColorIndex} = 25
+;
                                                         }

                                                            $worksheet
+->Cells($subDataRow, $subDataCol)->{Value} = $val6;
                                                        $subDataCol++;
                                                                     }
                                            $subDataCol=1;
                                                             }
                                                     }

                                                     else{  # aoh5 is 
+not an array

                                                     #################
+##
                                                     # MAIN INFORMATIO
+N#
                                                     #################
+##


                                                         # Index, if a
+ny of those, will be placed in first column
                                                         if ($sub5 eq 
+"number" || $sub5 eq "id" || $sub5 eq "enclosureNumber"){
                                                             $workshee
+t->Cells($mainHeaders, 1)->{Value} = $sub5;
                                            $worksheet->Cells($mainHea
+ders, 1)->{Font} -> {ColorIndex} = 25;
                                            $worksheet->Cells($mainHea
+ders, 1)->{Interior} -> {ColorIndex} = 15;
                                            $worksheet->Cells($mainHea
+ders, 1)->Borders() -> {Weight} = 2;
                                            $worksheet->Cells($mainHea
+ders, 1)->{Font}->{Bold} = 1;
                                                      $worksheet->Cell
+s($rowCount, 1)->{Value} = $aoh5;
                                            $worksheet->Cells($rowCoun
+t, 1)->{Font}->{Bold} = 1;
                                            $worksheet->Cells($rowCoun
+t, 1)->Borders() -> {Weight} = 2;
                                                         }

                                                         else{
                                                             unless ($
+worksheet->Cells($mainHeaders, $colCount)->{Value}){

                                                                 $work
+sheet->Cells($mainHeaders, $colCount)->{Value} = $sub5;
                                                        }

                                                    if ($sub5 eq "wwpn
+"){
                                                $worksheet->Cells($row
+Count, $colCount)->{NumberFormat} = "0";
                                                $worksheet->Cells($row
+Count, $colCount)->Borders() -> {Weight} = 2;

                                                    }

                                                             $workshee
+t->Cells($rowCount, $colCount)->Borders() -> {Weight} = 2;
                                                             $workshee
+t->Cells($rowCount, $colCount)->{Value} = $aoh5;
                                                             $workshee
+t->Cells($mainHeaders, $colCount)->Borders() -> {Weight} = 2;
                                            $worksheet->Cells($mainHea
+ders, $colCount)->{Font}->{Bold} = 1;
                                            $worksheet->Cells($mainHea
+ders, $colCount)->{Font} -> {ColorIndex} = 25;
                                            $worksheet->Cells($mainHea
+ders, $colCount)->{Interior} -> {ColorIndex} = 15;
                                            $colCount++;
                                            $LastRow = $worksheet->Use
+dRange->Find({What=>"*", SearchDirection=>xlPrevious, SearchOrder=>xl
+ByRows})->{Row};
                                        }
                                    }
                                } $rowCount++;
                                  $subDataRow++;
                                  $colCount=2;
                                  $worksheet -> Range("A:X") -> {Colum
+ns} -> Autofit;

                            }
                              $subDataRow=$rowCount+2;;
                              $colCount=2;
                              $rowCount=4;

                        #Seperate general information on first sheet.
                        } $rowCount=4; $colCount+=3;
                    }
                } $rowCount=4; $colCount+=3;
            }
        } $rowCount=4; $colCount+=3;
    }
}
[download]

NOTE: I noticed after i wrote it out that the while loses the sort keys functionality you had. If that's important, you can replace while( my ($sub2, $aoh2) = each %$sub2Hash ){ with:

  foreach my $sub2 ( sort keys %$sub2Hash ){
    my $aoh2 = $sub2Hash->{$sub2};
[download]

Note: i also feel like there should be a much more elegant (& shorter/less cumbersome) solution maybe recursing through those 6 levels .. at least through the first ~4 or 5. (don't have time at the moment to write it out)

In Section Seekers of Perl Wisdom