You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2013/07/31 05:07:32 UTC
[Bug 55330] New: Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
Bug ID: 55330
Summary: Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Product: POI
Version: 3.9
Hardware: All
OS: All
Status: NEW
Severity: enhancement
Priority: P2
Component: POI Overall
Assignee: dev@poi.apache.org
Reporter: romeara@live.com
(Filed under POI Overall component as this applies to the user model shared by
both HSSF and XSSF)
There are various API definitions in interfaces from the
org.apache.poi.ss.usermodel package which could be made significantly more
type-safe by replacing short/int arguments with various Java enumerations. Some
of these methods have been type safe for particular implementations in a way
which could be expanded to include both implementations (both meaning the xssf
and hssf implementations of the interfaces)
The most prominent example(s) of this are (all from the
org.apache.poi.ss.usermodel package):
- Cell
- get/setCellType
- CellStyle
- get/setAlignment
- get/setVerticalAlignment
- get/setBorderLeft
- get/setBorderRight
- get/setBorderTop
- get/setBorderBottom
- get/setFillPattern
Font
- get/setTypeOffset
- get/setUnderline
- get/setCharSet
- get/setBoldweight
Sheet
- get/setMargin
The proposal for this enhancement is to create/fully utilize enumerations for
the various sets of numeric constants which are meant to be used with these
methods to create alternate APIs, and to deprecate the APIs which are not type
safe in favor of the new ones.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
--- Comment #2 from Nick Burch <ap...@gagravarr.org> ---
Two comments and one query:
- Before defining the Enum, there'd need to be some time spent looking over
the File Format Documentation to ensure that all the different options are
actually defined, and none have been missed
- As well as this, there would need to be a check done of all the test
documents we have to ensure that no extra values in common use
- How would we handle a file where the value written in it when reading wasn't
one of the expected ones? (Could be slightly invalid, from a different program,
reserved value in use etc - real world files are often messy!)
It might be worth picking one of the smaller, more self contained cases from
the list, and trying to work up a patch for just that. A patch will mean code
to review and discuss (rather than just in the abstract), and would offer more
of a chance to see the best ways to handle the comments+question above.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
--- Comment #3 from Ryan O'Meara <ro...@live.com> ---
I meant to ask if you'd want to see a patch for all or a patch for one to see
if you liked how I went about it - guess that answers that question! I will
work up a patch for one of the better defined cases (I'm thinking margins -
there are only so many valid options for that one) and we can discuss based on
that.
There are a couple of different techniques I'd suggest for the invalid value
scenario - we can either include an "unknown value" enum (throwing an exception
if its attempted as an input) to cover those cases, or we can throw an
exception during parsing if we are confident that we have covered all values.
It shouldn't be too hard t nail down the values for most of these - they
generally say right in the docs "must be on of (list of constants)", which is
what made me think enumerations would be a better fit for this area. If the
function is only meant to accept a certain range of values, it would be better
to use Java's built-in type safety to help support it instead of accidentally
letting values through or throwing exceptions at run time.
So, current plan:
- Scrub the file format documentation for the possible valid margin arguments
- Check the example/test documents for commonly used margin values
- Create an enumeration of those values, containing the data necessary to
convert them to the file format data
- Create a patch and attach to this bug for discussion
By the way, at the moment, I have checked out the source using Git. Will a
patch created via Git work, or should I check out the source via SVN and create
a patch using that?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
Ryan O'Meara <ro...@live.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |romeara@live.com
--- Comment #1 from Ryan O'Meara <ro...@live.com> ---
I am willing to work on creating a patch for this enhancement if there is
general agreement it is a desired enhancement
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
--- Comment #7 from Ryan O'Meara <ro...@live.com> ---
Created attachment 30653
--> https://issues.apache.org/bugzilla/attachment.cgi?id=30653&action=edit
Page Margins Enumeration
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
--- Comment #5 from Ryan O'Meara <ro...@live.com> ---
OK, sounds good. My planned commit size was one commit per type enumerated (for
example, margins) including the added enumeration, modifications, and any unit
tests.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
--- Comment #4 from Nick Burch <ap...@gagravarr.org> ---
(In reply to Ryan O'Meara from comment #3)
> By the way, at the moment, I have checked out the source using Git. Will a
> patch created via Git work, or should I check out the source via SVN and
> create a patch using that?
The canonical source tree for Apache POI is the SVN one. The git repo is a
read-only copy.
Any patches will need to be applied using SVN, but unless you're doing
something unusual a diff generated by Git is processable by modern copies of
SVN. Please send in single git patches for meaningful chunks - you may need to
squash some smaller commits into bigger ones so we end up with things of a
suitable size to review and apply
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] [PATCH] Enhance Type Safety of
org.apache.poi.ss.usermodel Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
Nick Burch <ap...@gagravarr.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable
Summary|Enhance Type Safety of |[PATCH] Enhance Type Safety
|org.apache.poi.ss.usermodel |of
|Interface APIs |org.apache.poi.ss.usermodel
| |Interface APIs
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 55330] Enhance Type Safety of org.apache.poi.ss.usermodel
Interface APIs
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=55330
--- Comment #6 from Ryan O'Meara <ro...@live.com> ---
Here is a patch which adds type safe operations for page margins. I started
with margins as the constants used there do not go directly into the generated
file - they are just used to determine which function is called on internal
handler routines, so there is less risk of unanticipated values which need to
be handled by the APIs
The patch adds an enumeration and the new API calls, deprecates the old API
calls and constants, and duplicates the tests which existed for the old API
calls for the new ones
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org