Bug 58636 - Upgrade ClientAnchor.get/setAnchorType to use enums
Summary: Upgrade ClientAnchor.get/setAnchorType to use enums
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: 3.14-dev
Hardware: PC Linux
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks: 59836 59907
  Show dependency tree
 
Reported: 2015-11-23 06:16 UTC by Javen O'Neal
Modified: 2016-09-14 05:34 UTC (History)
0 users



Attachments
Add AnchorType enum inside ClientAnchor (26.98 KB, patch)
2015-11-24 11:03 UTC, Javen O'Neal
Details | Diff
Add AnchorType enum inside ClientAnchor (26.98 KB, patch)
2015-11-24 11:05 UTC, Javen O'Neal
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Javen O'Neal 2015-11-24 11:03:50 UTC
Created attachment 33291 [details]
Add AnchorType enum inside ClientAnchor

I'd like to minimize the code impact on existing projects, but I wasn't able to figure out a way to make enums work while allowing the following contrived example code to not break:
if (anchor.getAnchorType() != ClientAnchor.MOVE_AND_RESIZE) {
    anchor.setAnchorType(ClientAnchor.MOVE_AND_RESIZE);
}

Because the enum is a class inside the ClientAnchor, the above code would need to be rewritten as 
if (anchor.getAnchorType() != AnchorType.MOVE_AND_RESIZE) {
    anchor.setAnchorType(AnchorType.MOVE_AND_RESIZE);
}

Existing code using int literals rather than the named constant will break, and I don't have a problem with that.

Please look over ClientAnchor.java, XSSFClientAnchor.java, and HSSFClientAnchor.java, the first 3 files in the attached patch, to let me know how this could be better implemented.

Otherwise, I'll assume this is good and deploy it.
Comment 2 Javen O'Neal 2015-11-24 11:05:19 UTC
Created attachment 33292 [details]
Add AnchorType enum inside ClientAnchor
Comment 3 Nick Burch 2015-11-24 18:40:15 UTC
For backwards compatibility, couldn't you alias in the constants to ClientAnchor with something like this?

@Deprecated
public static final AnchorType MOVE_AND_RESIZE = AnchorType.MOVE_AND_RESIZE;
Comment 4 Javen O'Neal 2015-11-25 07:24:59 UTC
(In reply to Nick Burch from comment #3)
> @Deprecated
> public static final AnchorType MOVE_AND_RESIZE = AnchorType.MOVE_AND_RESIZE;

Thanks. That answers the remaining questions/discomfort with what I had.

Added in r1716313. Updated docs in r1716314.
Comment 5 Javen O'Neal 2016-09-14 05:34:46 UTC
(In reply to Javen O'Neal from bug 59907 comment #8)
> In r1760617, I have restored ClientAnchor#setAnchorType(int) to regain
> backwards compatibility with POI 3.13 that was broken in POI 3.14 beta 1
> thru POI 3.15 final RC2. ClientAnchor#setAnchorType(int) will be available
> in 3.15 final RC3 and removed no earlier than 3.17.
> 
> I have opened discussion for how to handle getAnchorType() returns int or
> AnchorType enum on the dev mailing list. Reverting back to 3.13's behavior
> of returning an int would break backwards compatibility for those who
> upgraded to 3.14, and break compatibility when we switch to only supporting
> enums.