> On 2011-06-13 18:23:35, Matt Foley wrote:
> > Sorry if this is out of context, but is it really best to also return a
> > null here? Shouldn't it check for null result from getSerialization(),
> > then throw a (non-NPE) exception? Or do you prefer to do that check and
> > throw at a higher level of the code?
>
> Todd Lipcon wrote:
> I thought about this while doing the review... my thinking was that our
> other similar APIs do return null -eg
> CompressionCodecFactory.getCodecByName() returns null if the specified codec
> isn't found. This API is also marked as LimitedPrivate Evolving so it
> shouldn't be a problematic breaking change.
>
> Maybe we should add a bit of javadoc saying "@returns null if no
> serializer is known for the given class." ?
The public method getSerialization() returns a null if it does not find an
acceptor. I think it is fair that a get{Serializer,Deserializer}() call do the
same since they are specific nature calls underneath?
Or we could revamp the entire set if Exceptions are better to have over null
returns and null checks, but it ought to be consistent is what I think.
- Harsh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/884/#review817
-----------------------------------------------------------
On 2011-06-11 22:10:17, Harsh J wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/884/
> -----------------------------------------------------------
>
> (Updated 2011-06-11 22:10:17)
>
>
> Review request for hadoop-common and Todd Lipcon.
>
>
> Summary
> -------
>
> Since getSerialization() can possibly return a null, it is only right that
> getSerializer() and getDeserializer() usage functions do the same, instead of
> throwing up NPEs.
>
> Related issue to which this improvement is required:
> https://issues.apache.org/jira/browse/MAPREDUCE-2584
>
>
> This addresses bug HADOOP-7328.
> http://issues.apache.org/jira/browse/HADOOP-7328
>
>
> Diffs
> -----
>
> src/java/org/apache/hadoop/io/serializer/SerializationFactory.java dee314a
>
> Diff: https://reviews.apache.org/r/884/diff
>
>
> Testing
> -------
>
> Existing SequenceFile serialization factory tests pass. The change is merely
> to make the functions return null instead of throwing an NPE within.
>
>
> Thanks,
>
> Harsh
>
>
