Copy XLS File Then Delete All But One Worksheet?

FLX TMS FlexCel for VCL & FMX
1
I'd like to copy a TXLSFile that is in memory and then in the new version delete all but the active sheet. I thought this would be easy but I've run into a problem.

Here's a simplified version of the code (see below). The error message is "There are not visible sheets on the file". However, the sheet count is shown as 3.

Is this a bug? Here's a link to the small demo project: https://goo.gl/3wgnVp 

procedure TForm1.Button1Click(Sender: TObject);
var
	xls: TXLSFile;
	xlsErrors: TXLSFile;
  s: TMemoryStream;
  i, j: integer;
  LastBad: integer;
  RowStart, RowCount: integer;
begin
	//-- Load File
  xls := TXlsFile.Create;
  xls.Open('C:\Users\Steve Maughan\Documents\Embarcadero\Studio\Projects\Delete Sheets\Book1.xlsx');
  xls.ActiveSheet := 2;


	//-- Duplicate the workbook
  xlsErrors := TXLSFile.Create;
	s := TMemoryStream.Create;
  xls.Save(s);
  s.Position := 0;
  xlsErrors.Open(s);
  s.Free;


	//-- Delete all the other worksheets
  xlsErrors.ActiveSheet := xls.ActiveSheet;
  i := xlsErrors.SheetCount;
  j := xls.ActiveSheet;
  while xlsErrors.SheetCount > 1 do
  begin
  	if (i <> j) then
    	xlsErrors.DeleteSheet(i); //  <----ERROR ("There are not visible sheets on the file")
    dec(i);
  end;


  //-- Export
  xlsErrors.Save('ErrorFile.xlsx');
  xlsErrors.Free;
end;
2

The problem is with the definition of DeleteSheet. It can be very misleading, and I would change it if it wouldn't break all existing code, but as it would, we are stuck with it.


Basically, the argument to DeleteSheet is not the sheet to delete but the number of sheets to delete. DeleteSheet(3) will delete 3 sheets from the ActiveSheet, not delete sheet 3. When we introduced the API like 20 years ago it didn't look bad, today it does but we can't change it.

To make your code work, change the loop to be:


  while xlsErrors.SheetCount > 1 do
  begin
  	if (i <> j) then
    begin
      xlsErrors.ActiveSheet := i;
    	xlsErrors.DeleteSheet(1);
    end;
    dec(i);
  end;
3

And btw, ugly as it is, you could take advantage of the definition of DeleteSheet and remove the loop.

Just make ActiveSheet = 1 and DeleteSheet(xls.ActiveSheet - 1), then make ActiveSheet =2 and remove the rest of the sheets with other DeleteSheet(xlsError.SheetCount -1).

You need to account for the border cases where xls.ActiveSheet is 1 or xls.SheetCount, but it should work and it is simpler.  
4

Awesome - that worked!!


Maybe you can create another method "DeleteSheetByIndex"?

Thanks,

Steve 
5
You know, some weeks ago I was reading a discussion on the C++ groups on C++20 about how they are trying to decrease C++ complexity by adding more complexity. Here, at a much lower scale, something similar happens: By adding more methods, we might increase, not decrease the complexity.

But well, I've done the following:
1. I made a better error message. Instead of "there are no sheets visible on the file"  it will tell you now that you are trying to delete more sheets than the sheets in the file.

2. I've added 2 overloads:
2.1.  DeleteSheet(sheetindex, sheetcount)  which will work more like you expect it. The first parameter is the sheet to delete, the second how many sheets to delete.
  
2.2 DeleteSheet(sheetName). Just because it can be helpful.

The "right" solution here would be to deprecate DeleteSheet(count) and add leave the 2 new methods to be the right ones, but we try really hard to not deprecate stuff, unless it is really critical to do it. Just some hours ago I had an email from a customer upgrading from FlexCel.NET 5.3 (like about 7 years ago), and the update was just a recompile, even if the product has little in common with what it was 7 years ago. If when you try to update you find out a lot of deprecated stuff or methods that don't exist anymore, you are more likely to not upgrade and live forever with your version. And we really want to make the update simple, because we are constantly fixing stuff, and by updating we ensure you won't find bugs that were already fixed. So if we can avoid it, we try to not deprecate or remove any functionality.

Ps: A side note: I didn't look at the code much yesterday, but I think that you could achieve the same with InsertAndCopySheets? That is, instead of saving the file, opening it in a new object, and then delete all the other sheets, you can just copy a sheet from one file to another.

Take a look at the example:
http://www.tmssoftware.biz/flexcel/doc/vcl/samples/delphi/api/consolidating-files/index.html
6

Awesome!


Great approach (as always),

Thanks for the help,

Best regards,

Steve