Bug 46859 - [Patch] rename org.apache.poi.openxml4j.opc.Package
Summary: [Patch] rename org.apache.poi.openxml4j.opc.Package
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: 3.5-dev
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-15 23:33 UTC by Josh Micich
Modified: 2009-03-18 12:08 UTC (History)
0 users



Attachments
svn diff (165.65 KB, text/plain)
2009-03-15 23:44 UTC, Josh Micich
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Josh Micich 2009-03-15 23:33:05 UTC
Since the class java.lang.Package is available without import, failing to explicitly import org.apache.poi.openxml4j.opc.Package may cause confusing compiler errors.

As a general rule, POI classes should not have names that clash with any jdk-public (or other well known) class.

This is a change which would be good to get done before the 3.5-final release.
Comment 1 Josh Micich 2009-03-15 23:44:53 UTC
Created attachment 23386 [details]
svn diff

Please consider this patch and give feedback.

In this patch I have created a new base class called "OPCPackage".  The existing Package class has been deprecated and now extends OPCPackage. The rest of POI code has been changed to use the new non-deprecated class.

The new name 'OPCPackage' is negotiable.  Other names I considered were 'OpenPackage', 'OpenPackageContainer', 'XMLPackage' and 'PackageBase'.

If maintaining binary compatibility with previous poi-ooxml versions is important, we will need to re-add deprecated versions of many methods which take OPCPackage as a parameter.  Most of the usage of Package was internal, however there are some notable public methods/constructors, like XSSFWorkbook(Package).


An unrelated change was to restrict the class hierarchy of Package to only POI subclasses (i.e. ZipPackage).  This was done because POI discourages client classes from inheriting from POI classes.
Comment 2 Yegor Kozlov 2009-03-16 04:57:47 UTC
+1 to this change. 

I agree that name clash with java.lang.* classes is undesirable. However, it's normal to have collisions with other packages, namely, with java.awt.*.
There are Shape and Polygon objects both in HSLF and AWT and I think that they can co-exist happily. Even Java itself comes name clashes:
java.awt.Shape vs javax.swing.Shape, java.util.List vs java.awt.List, etc. 

As to binary compatibility, I think it's OK to require users to re-compile their applications while we are still in a beta phase. I'm very reluctant to pollute the code with overridden methods for the sake of binary compatibility: foo(Package pkg) and foo(OPCPackage pkg). 

Regards,
Yegor
Comment 3 Nick Burch 2009-03-17 05:18:10 UTC
I'm happy with going for the change, with no binary compatibility extra methods as we're in beta

I'd probably lean more towards OOXMLPackage, rather than OPCPackage, but that's only a slight preference
Comment 4 Josh Micich 2009-03-18 12:07:03 UTC
Applied in svn r755699
Comment 5 Josh Micich 2009-03-18 12:08:51 UTC
(In reply to comment #4)
Applied in svn r755699 .
(sorry somehow lost the last digit)