Bug 57495 - [PATCH] getTableArray method can not get 0 pos table
Summary: [PATCH] getTableArray method can not get 0 pos table
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XWPF (show other bugs)
Version: 3.11-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2015-01-26 03:07 UTC by zhangyu
Modified: 2017-02-22 02:49 UTC (History)
2 users (show)



Attachments
get*Array methods fix (4.82 KB, patch)
2015-01-30 11:19 UTC, lordscales91
Details | Diff
updated patch with some tests (6.66 KB, patch)
2015-01-31 12:31 UTC, lordscales91
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zhangyu 2015-01-26 03:07:06 UTC
source:
 if(pos >0 && pos <tables.size()){
   return tables.get(pos);
}
return null;



I think 【pos >= 0 】 is correct.
Comment 1 lordscales91 2015-01-29 22:11:20 UTC
I noticed that not just this method, other get*Array methods have the same issue. Why nobody has noticed a simple mistake like this? 
I will try to create a patch for this tomorrow, if nobody has done it already.
Comment 2 lordscales91 2015-01-30 11:19:55 UTC
Created attachment 32412 [details]
get*Array methods fix

Patch added, I think it's ok, it's my first time creating a patch, I used the patch.xml Ant script provided, so it should be fine.

I also added a helper method in org.apache.poi.util.Units to convert pixels to EMUs
Comment 3 Nick Burch 2015-01-30 11:37:27 UTC
Thanks!

Any chance of some unit tests to go with this? That'll let us ensure it doesn't accidentally get broken in the future
Comment 4 lordscales91 2015-01-31 10:49:02 UTC
(In reply to Nick Burch from comment #3)
> Thanks!
> 
> Any chance of some unit tests to go with this? That'll let us ensure it
> doesn't accidentally get broken in the future

I can try, but don't promise nothing, it has been a long time since the last unit testing that I did. I'm not so much friend of unit tests, I'm more of printing traces to the command line XD. Of course I'm aware that for large projects like this one unit testing is much better. I will make a try
Comment 5 lordscales91 2015-01-31 12:31:22 UTC
Created attachment 32419 [details]
updated patch with some tests

Ok, I have successfully added some unit tests, however I couldn't test the methods on HeaderFooter and Footnote since I didn't found any easy straight-forward way of creating them, seems like you need to struggle with the internal Xml Beans.

Also, technically it's possible to create nested Tables in a cell, but there isn't any createTable method, hence I couldn't test the getTableArray method of the TableCell
Comment 6 Dominik Stadler 2016-03-12 11:46:43 UTC
Applied via r1734694, thanks for the Patch!
Comment 7 lordscales91 2016-03-13 10:16:03 UTC
(In reply to Dominik Stadler from comment #6)
> Applied via r1734694, thanks for the Patch!

Jesus! That took so long! But I'm glad to see that my contribution was useful.
Comment 8 Dominik Stadler 2016-03-13 12:20:32 UTC
As the saying goes "Gut Ding will Weile haben.", or in English "Haste makes waste." ;), or rather only a few people looking at a large pile of bug-reports...