|Summary:||[PATCH]HSSFWorkbook#cloneSheet() throws RuntimeException|
|Product:||POI||Reporter:||Toshiaki Kamoshida <kamoshida.toshiaki>|
|Component:||HSSF||Assignee:||POI Developers List <dev>|
|Severity:||normal||CC:||avernet, dennis, ndz43wv02|
To Fix simply is,Record implements Cloneable.
This is minimum testcase to "find" this problem,but not enough for this patch...
Added clone Method for org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate
Description Toshiaki Kamoshida 2002-12-04 05:27:11 UTC
Hello, When I tried to clone a sheet by calling HSSFWorkbook#cloneSheet(int), HSSFWorkbook#cloneSheet(int) throws RuntimeException from org.apache.poi.hssf.record.Record#clone(),because some subclasses of org.apache.poi.hssf.record.Record has no individual implementation of clone() method.
Comment 1 Toshiaki Kamoshida 2002-12-04 05:30:00 UTC
Created attachment 4036 [details] To Fix simply is,Record implements Cloneable.
Comment 2 Toshiaki Kamoshida 2002-12-04 05:45:42 UTC
Cloneable is very convenience. If one subclass's instance has only primitive or immutable object's reference fields,it is no need to override clone() method,each fields are automatically copied on native method at java.lang.Object,and there is no other bad effects.
Comment 3 Avik Sengupta 2002-12-12 08:43:59 UTC
1. Can u please supply a simple testcase for this error? 2. Anybody has any objections to my applying this? I tend to agree that Record should implement cloneable, but am not very conversant with the cloneSheet implementation (All the more reason why there should be a test case!!)
Comment 4 Toshiaki Kamoshida 2002-12-12 10:13:15 UTC
Created attachment 4140 [details] This is minimum testcase to "find" this problem,but not enough for this patch...
Comment 5 Toshiaki Kamoshida 2002-12-12 10:51:01 UTC
When I set a breakpoint at org.apache.poi.hssf.record.Record#clone(), and execute cloneSheet() by using "My" Excel file on a debuggar,I found there is no implementation of clone() at thease classes.If the target sheet has one of them,cloneSheet() will fail. 1. org.apache.poi.hssf.record.MargeCellsRecord 2. org.apache.poi.hssf.record.ColumnInfoRecord 3. org.apache.poi.hssf.record.BottomMarginRecord 4. org.apache.poi.hssf.record.LeftMarginRecord 5. org.apache.poi.hssf.record.RightMarginRecord 6. org.apache.poi.hssf.record.TopMarginRecord Maybe some other classes are same,I don't know. I feel,the testcase for this patch should use a sheet that has at least one instance of all each Record's subclass.But I can't create it easily. Please someone make it:)
Comment 6 Avik Sengupta 2002-12-12 12:49:36 UTC
thanks. thats what i wanted .. to be sure i am not solving the wrong problem. I tend to agree that getting Record to implement cloneable should be a simple fix without too many side effects. I'll wait a while to see if Jason (who implemented most of the cloning stuff) has any alternate ideas, and then go ahead (hopefully with some unit tests)
Comment 7 Dennis Doubleday 2002-12-12 14:17:09 UTC
This is a duplicate of #14487, which I created. However, this one now has more info, so if you want to mark #14487 as the duplicate, feel free.
Comment 8 Jason Height 2002-12-12 20:41:20 UTC
OK I originally wrote the cloneSheet implementation. The reason for not implementing clonable was that at the time I did not know which records were sheet based and which are workbook based. As such i wanted to know which records i still needed to implement. Instead of making Record implement Clonable it may be better to fix the records that dont have their own clone method. Initially there was some debate whether a shallow clone is appropriate (ie by implementing Clonable), and the suggestion from the list (Andy?) was to perform deep clones, which is how it is done. Jason
Comment 9 Jason Height 2002-12-12 20:53:16 UTC
Oh I forgot to mention, I did initially implement shallow clones and that worked fine. If my memory serves me its only the aggregate records that need to be deep cloned. I can implement the extra Records listed in this bug report next week if you want, or we can just implement Clonable and hope for the best. Jason
Comment 10 Avik Sengupta 2002-12-13 04:49:15 UTC
*** Bug 14487 has been marked as a duplicate of this bug. ***
Comment 11 Avik Sengupta 2002-12-13 04:53:15 UTC
I'll only be able to look at it next weekend, so if you can do something next week, go ahead.
Comment 12 Andy Oliver 2002-12-27 04:58:06 UTC
Shawn does this work?
Comment 13 Avik Sengupta 2003-02-27 13:32:54 UTC
*** Bug 17474 has been marked as a duplicate of this bug. ***
Comment 14 Avik Sengupta 2003-03-01 14:30:15 UTC
*** Bug 17540 has been marked as a duplicate of this bug. ***
Comment 15 Avik Sengupta 2003-03-01 14:32:09 UTC
fixed, afaik, with all records mentioned herein.
Comment 16 Henning Boeger 2003-03-05 16:51:44 UTC
This class needs a clone() Method too: org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate
Comment 17 Henning Boeger 2003-03-20 10:20:56 UTC
Created attachment 5441 [details] Added clone Method for org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate
Comment 18 Avik Sengupta 2003-03-20 20:03:51 UTC
Modified version comitted (made it deeper clone). Thanks Henning. Pls cross check.