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 2016/04/16 05:25:33 UTC
[Bug 59336] New: [patch] Deprecate
CellUtil.setCellStyleProperty(Cell, Workbook, String, Object)
https://bz.apache.org/bugzilla/show_bug.cgi?id=59336
Bug ID: 59336
Summary: [patch] Deprecate CellUtil.setCellStyleProperty(Cell,
Workbook, String, Object)
Product: POI
Version: 3.15-dev
Hardware: PC
Status: NEW
Severity: normal
Priority: P2
Component: SS Common
Assignee: dev@poi.apache.org
Reporter: jmarkmurph@yahoo.com
Created attachment 33770
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33770&action=edit
Patch file generated via Ant script
Due to addition of CellUtil.setCellStylePrperties(Cell, Map<String, Object>),
the Workbook parameter of CellUtil.setCellStyleProperty(Cell, Workbook, String,
Object) does nothing except throw an exception if it does not match the
Workbook object associated with Cell. This patch deprecates
CellUtil.setCellStyleProperty(Cell, Workbook, String, Object) in favor of
CellUtil.setCellStyleProperty(Cell, String, Object). Objects using the
deprecated method are modified appropriately to use the new method. One aside,
HSSFCellUtil is nothing but a wrapper for some of the methods in CellUtil. No
XSSFCellUtil class exists. It is my opinion that the entire class HSSFCellUtil
should be deprecated in favor of CellUtil, or it should be completed and
XSSFCellUtil added. I prefer the former as HSSFCellUtil gives us nothing except
the "ability" to lock the user in to the older HSSF format.
--
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 59336] [patch] Deprecate CellUtil.setCellStyleProperty(Cell,
Workbook, String, Object)
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59336
--- Comment #2 from Javen O'Neal <on...@apache.org> ---
(In reply to Mark Murphy from comment #0)
> It is my opinion that the entire class HSSFCellUtil should be deprecated in
> favor of CellUtil
I agree. Please open a separate bug to deprecate features in HSSFCellUtil and
move them to CellUtil if CellUtil does not provide the same functionality.
class HSSFCellUtil {
public static Object someMethodThatIsAlsoInCellUtil() {
System.out.println("Hello World");
return null;
}
}
becomes
class HSSFCellUtil {
/*
* @deprecated 3.15 beta2. Removed in 3.17. Use {@link
org.apache.poi.ss.util.CellUtil@someMethodThatIsAlsoInCellUtil} instead.
*/
public static Object someMethodThatIsAlsoInCellUtil() {
return CellUtil.theMethodInCellUtil();
}
}
--
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 59336] [patch] Deprecate CellUtil.setCellStyleProperty(Cell,
Workbook, String, Object)
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59336
Mark Murphy <jm...@yahoo.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #33770|0 |1
is obsolete| |
--- Comment #3 from Mark Murphy <jm...@yahoo.com> ---
Created attachment 33771
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33771&action=edit
Patch file generated via Ant script
You are right, that was the wrong patch file. Try this one.
--
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 59336] [patch] Deprecate CellUtil.setCellStyleProperty(Cell,
Workbook, String, Object)
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59336
Javen O'Neal <on...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #4 from Javen O'Neal <on...@apache.org> ---
Thanks for the patch!
Applied with minor modification in r1739533 and r1739536. Updated changelog in
r1739537.
Minor modifications:
TestCellUtil#setCellStyleProperties: assertEquals(styCnt1 + 1, styCnt2);
For helpful error messages when a junit assertion fails, the order of the
parameters should be: assertEquals(expected, actual) or assertEquals(message,
expected, actual). In this case, we expect 1 additional style to be created
(styCnt+1), and the actual is styCnt2 (wb.getNumCellStyles() called after the
cell style is added to the style table).
I'm not too worried about line width. The coding style for this project aims
for 80-100 characters width. I interpret this as "go over this limit when it
improves readability". With the ubiquity of widescreen monitors, I assume that
most developers looking at the code have screen space for 160-240 characters. I
reverted some of your line-width/whitespace changs when I felt it didn't
significantly improve the readability of the code.
https://poi.apache.org/guidelines.html#CodeStyle
Indentation: prefer 4 spaces over tabs, but better to be consistent in the
method or class if a different indentation scheme is used. I replaced some tabs
with spaces to make them consistent with the rest of the file.
When deprecating methods, try to write the deprecated method in terms of the
non-deprecated method to reduce code duplication (in case there's a bug in the
non-duplicated method that gets fixed, you want the deprecated method to also
get fixed) and for reducing the total number of lines of code--even if this
makes the method slightly more expensive by recomputing values. See the change
I made to CellUtil#setFont(Cell, Workbook, Font). This also makes it easier for
developers to see how to update their code to the non-deprecated method.
To check your javadocs, run "ant javadocs" (runs in under 1 minute). This
caught a copy-paste error with {@link #setFont(Cell, short)}.
--
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 59336] [patch] Deprecate CellUtil.setCellStyleProperty(Cell,
Workbook, String, Object)
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59336
--- Comment #1 from Javen O'Neal <on...@apache.org> ---
Did you attach the right patch? This patch looks like the patch for bug 58787.
Usually "deprecation bugs" are just a few @deprecated annotations and replacing
the usage of deprecated functions in the code and unit tests with
non-deprecated versions.
--
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 59336] [patch] Deprecate CellUtil.setCellStyleProperty(Cell,
Workbook, String, Object)
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=59336
Mark Murphy <jm...@yahoo.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |enhancement
OS| |All
--
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