| Summary: | [patch] Deprecate CellUtil.setCellStyleProperty(Cell, Workbook, String, Object) | ||
|---|---|---|---|
| Product: | POI | Reporter: | Mark Murphy <jmarkmurph> |
| Component: | SS Common | Assignee: | POI Developers List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | ||
| Priority: | P2 | ||
| Version: | 3.15-dev | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | All | ||
| Attachments: |
Patch file generated via Ant script
Patch file generated via Ant script |
||
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. (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(); } } Created attachment 33771 [details]
Patch file generated via Ant script
You are right, that was the wrong patch file. Try this one.
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)}. |
Created attachment 33770 [details] 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.