sharief has asked for the wisdom of the Perl Monks concerning the following question:

Hello respected monks, I would like to delete a set of excel worksheets. My code is this

use Spreadsheet::ParseExcel; use Win32::OLE; use Win32::OLE::Const 'Microsoft Excel'; my $xl = Win32::OLE->new('Excel.Application'); $ex_cop ="C:\\Perl\\Copy of path1.xls"; ##Here i read the excel file my $Books = $xl->Workbooks->Open($ex_cop); ##Here i read the same excel file in order to take the total number of + worksheets count my $oBook = Spreadsheet::ParseExcel::Workbook->Parse($ex_cop); my $worksheet_count=$oBook->{SheetCount}; ##This array has excel sheet names which has to be deleted my @excel_del_pages=("del","Sheet1","Sheet2","Sheet3"); foreach $ex_del(@excel_del_pages) { for($i=1;$i<=$worksheet_count;$i++) { $sheetname2= $Books->Worksheets($i)->{name}; if($sheetname2->{name} eq $ex_del) { $Books->Worksheets($i)->Delete(); } } }

error in cmd prompt is Can't use an undefined value as a HASH reference at please_delete.pl line 27. i.e here $sheetname2= $Books->Worksheets($i)->{name};

Need Help

I get names of excel sheets if i print them using code

foreach $ex_del(@excel_del_pages) { for($i=1;$i<=$worksheet_count;$i++) { $sheetname2= $Books->Worksheets($i)->{name}; print $sheetname2; if($sheetname2->{name} eq $ex_del) { $Books->Worksheets($i)->Delete(); } } }

I get del as output which is the first sheet name for print statement

Replies are listed 'Best First'.
Re: Excel worksheet delete
by roboticus (Chancellor) on Jan 18, 2012 at 17:29 UTC

    sharief:

    The problem is that your call to $Books->Worksheets($i) is returning an undef, so the remainder of the line looks kinda like undef->Delete() which is nonsensical. Check your return value from your Worksheets call, and perhaps run under the debugger (perl -d foo.pl) so you can inspect values and figure out what's going on inside your code.

    ...roboticus

    When your only tool is a hammer, all problems look like your thumb.

Re: Excel worksheet delete
by davies (Monsignor) on Jan 18, 2012 at 20:34 UTC

  • What's wrong with your question?

    Your code is not indented. This makes it very hard to read.

    Your code won't run without your file. This makes it impossible for anyone to reproduce your problem.

  • What's wrong with your code?

    There's no need to differentiate in code between for and foreach. Perl will work out which you mean.

    You use ParseExcel, but apparently only to find out how many sheets there are. This is very inefficient.

    You go through the sheets from 1 to n. This is always dangerous. See RFC Tutorial: Adding and extracting VBA to and from Excel files comment 1 and Re: Delete Row in ExcelRFC Tutorial - Deleting Excel Rows, Columns and Sheets, which describes the issues specifically.

    You have a loop within a loop to match sheet & array names. This is inefficient, making your code O(n^2). My code isn't much better on this - more later.

    You mix the terms "sheet" and "page". A page in Excel is something that gets printed. I know it's confusing because they refer to "workbooks", but the more strictly you use terminology, the easier it is to answer questions.

    You don't use strict & warnings.

  • What would work?

    use strict; use warnings; use Win32::OLE; my $xl = Win32::OLE->new('Excel.Application'); $xl->{Visible} = 1; $xl->{SheetsInNewWorkbook} = 5; my $wb = $xl->Workbooks->Add; $wb->Sheets(4)->{Name} = 'del'; $wb->Sheets(5)->{Name} = 'dell'; my @excel_del_sheets = ("del", "Sheet1", "Sheet2", "Sheet3"); for (my $shtNo = $wb->Sheets->{Count}; $shtNo > 0; $shtNo--) { my $shtName = $wb->Sheets($shtNo)->{Name}; if (grep(/^$shtName$/, @excel_del_sheets)) { $wb->Sheets($shtNo)->Delete; } }

  • How does it work?

    First, lines 12 to 18 can be commented out and the code run without them. This will set up a file so that the user can see that the previous code works.

    Now we loop through each sheet starting at the end. If it is found in the array, it is deleted.

    Sheet 5 is named "dell" so that we can prove that subsets of names are not deleted. This is the purpose of the "^" and "$" in the grep regex.

  • How could it be improved?

    After a sheet has been deleted, delete the name from the array. Then future searches will be quicker. Is it worth it? For a few sheets, no. For hundreds of files with hundreds of sheets, certainly.

    The code should check that it is not deleting the last sheet in a workbook. Excel can't do this & the Perl will crashsilently fail to delete the sheet.

    Regards,

    John Davies