Bug 58254 - [PATCH] Inconsistent documentation with CellFormatResult.text
Summary: [PATCH] Inconsistent documentation with CellFormatResult.text
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2015-08-18 04:19 UTC by Javen O'Neal
Modified: 2015-08-19 18:00 UTC (History)
3 users (show)

Fixed documentation patch (490 bytes, patch)
2015-08-18 18:30 UTC, Javen O'Neal
Details | Diff
throws IllegalArgumentException patch (2.72 KB, patch)
2015-08-18 18:33 UTC, Javen O'Neal
Details | Diff
throws IllegalArgumentException patch (2.98 KB, patch)
2015-08-18 18:38 UTC, Javen O'Neal
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Javen O'Neal 2015-08-18 04:19:31 UTC
According to the JavaDocs, CellFormatResult.text may never be null. However, there's nothing in the constructor that guarentees this.

Either the documentation needs to be updated or the constructor needs to throw an IllegalArgumentException if the passed-in text parameter is null.IllegalArgumentException
Comment 1 Nick Burch 2015-08-18 15:00:29 UTC
I wonder if throwing a NullPointerException would be better than IllegalArgumentException?

If you could work up a unit test + patch for whatever seems the most sensible option, that'd be wonderful!
Comment 2 Javen O'Neal 2015-08-18 18:30:06 UTC
Created attachment 33009 [details]
Fixed documentation patch
Comment 3 Javen O'Neal 2015-08-18 18:33:25 UTC
Created attachment 33010 [details]
throws IllegalArgumentException patch

Select one of the attached patches, NOT BOTH!

Feel free to change the exception type to NullPointerException. I feel that an IllegalArgumentException is more descriptive because it describes the problem as invalid input to a method/constructor rather than accidentally dereferencing a null pointer (which would happen if the constructor didn't check for null and a later method expected non-null).
Comment 4 Javen O'Neal 2015-08-18 18:38:31 UTC
Created attachment 33011 [details]
throws IllegalArgumentException patch

import java.awt.Color instead of java.awt.*
Comment 5 Nick Burch 2015-08-19 18:00:58 UTC
Thanks, second patch committed in r1696638.