On 15 February 2014 23:14, Bernd Eckenfels <[email protected]> wrote:
> Hello,
>
> (pruning conversation)
>
> Am Sat, 15 Feb 2014 21:43:36 +0000 schrieb sebb <[email protected]>:
>> On 15 February 2014 21:29, Bernd Eckenfels <[email protected]>
>> > I also wonder if this is really a
>> > good idea to read the data fully into memory, only to stream it
>> > then to somewhere else.
>>
>> The method in question returns the content of the file as an array of
>> bytes. It is a public method, so the API cannot readily be changed.
>
> True (luckily it gets internally only used in the write() method when the 
> inMemory case is hit).
>
> But I suggest to add some warning to the 3 methods get/getString (the class 
> javadoc mentions the problem but it is better to repeat it):
>
> Also the null return on IO errors is unfortunate, but since it cannot easily 
> be changed, it should at least be documented:
>
> 295     /**
> 296     * Returns the contents of the file as an array of bytes. If the
> 297     * contents of the file were not yet cached in memory, they will be
> 298     * loaded from the disk storage and cached.
> 299     *
> 300     * @return The contents of the file as an array of bytes.
> 301     */
>
> new:
>
> 295     /**
> 296     * Returns the contents of the file as an array of bytes. If the
> 297     * contents of the file were not yet cached in memory, they will be
> 298     * loaded from the disk storage and cached. Be careful in requesting
>         * the in-memory representation for larger files.
> 299     *
> 300     * @return The contents of the file as an array of bytes. This might
>         *   return <code>null</code> in case of I/O Exceptions while reading
>         *   the data from disk.
> 301     */
>
> Does that require a JIRA/Changelog entry or can I commit that without?

I don't think it's usually necessary to create JIRAs for Javadoc clarifications.

> The getString() and getString(String) methods do not handle the above 
> mentioned null case. At least we should document that it can throw a NPE, but 
> most likely thats not a good thing to keep (no matter how compatible or not).

The class does not handle null fields well; there are lots of cases
where NPEs can occur if methods are not called in the expected order
(FILEUPLOAD-247)

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to