Bug 51175 - [Patch] Proposal: Avoid Autoboxing
Summary: [Patch] Proposal: Avoid Autoboxing
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XWPF (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-09 09:41 UTC by Stefan Stern
Modified: 2011-05-13 12:22 UTC (History)
0 users



Attachments
Converts all occurrences of autoboxing / auto-unboxing into explicit conversions. (9.39 KB, application/octet-stream)
2011-05-09 09:41 UTC, Stefan Stern
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Stern 2011-05-09 09:41:26 UTC
Created attachment 26976 [details]
Converts all occurrences of autoboxing / auto-unboxing into explicit conversions.

As Autoboxing may cause some trouble during runtime, I propose to avoid it and use explicit conversions where needed. An example line of code for misunderstandings is this:

[code]
Integer index = 5;
List<ExampleObjects> list = getExampleList();
list.remove(index);
[/code]

One might expect to have element at index 5 removed from the list. But the API will call the method with signature List#remove(Object), so nothing happens. 

If reading the code in terms of another issue, one can easily miss the difference and wonder why on earth the element at index 5 was not removed. 

Therefore, I added explicit conversion whereever I found autoboxing takes place in the XPWF classes. This includes a minor API change for two methods in XWPFDocument, where "Integer" was used as return type, while "int" is sufficient and fits better into the rest of the get-index-methods of XWPF.

Of course, this is a matter of coding style. So this just a proposal.
Comment 1 Nick Burch 2011-05-09 10:49:38 UTC
I agree we should probably avoid returning Integer or Boolean from methods wherever possible. Possibly within the code too, but List<Integer> interchanging with int for example is one case where we probably don't want too much explicit code cluttering up.

I'll try to review the patch shortly.
Comment 2 Stefan Stern 2011-05-09 11:45:04 UTC
Whether to use or not to use Autoboxing, that is one of those issues that turn out to become religious conflicts. After I stumbled over the List API, I decided to avoid them. But yes, it tends to inflate the code. 

Just keep in mind, how easily the following lines can be misunderstood:

[code]
List<Integer> list = new ArrayList<Integer>();

list.add(0);
list.add(1); 
list.add(2);
list.add(3);
list.add(2,4);
list.add(7);

list.remove(0);
list.remove(3);
list.remove(new Integer(3));
list.remove(7);
[/code]

Just my 2 cents. ;)
Comment 3 Nick Burch 2011-05-13 12:22:52 UTC
Applied (with a few tweaks) in r1102691, thanks!