Bug 58636

Summary: Upgrade ClientAnchor.get/setAnchorType to use enums
Product: POI Reporter: Javen O'Neal <onealj>
Component: SS CommonAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: enhancement    
Priority: P2    
Version: 3.14-dev   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 59836, 59907    
Attachments: Add AnchorType enum inside ClientAnchor
Add AnchorType enum inside ClientAnchor

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.