commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Benson <gudnabr...@gmail.com>
Subject Re: [discuss][Digester] The future of Digester an the Digester3 in sandbox
Date Tue, 01 Mar 2011 18:02:40 GMT

On Mar 1, 2011, at 10:22 AM, sebb wrote:

> On 1 March 2011 15:55, Simone Tripodi <simonetripodi@apache.org> wrote:
>> Hi Seb!
>> thanks a lot for your hints too, very appreciated!
>> 
>> Something suggests me to remove the @SuppressWarnings because
>> ClassCastException are admitted, let's suppose I have the given XML:
>> 
>> <a>
>>    <prop>text</prop>
>> </a>
>> 
>> that can be mapped to
>> 
>> class A {
>>    String prop;
>> }
>> 
>> if a user tries to map to a different type
>> 
>> B b = digester.parse("<a><prop>text</prop></a>");
>> 
>> An exception has to be thrown.
> 
> It will be thrown with the generic fix, because the compiler adds a
> cast to the parse return.
> 
> However, there is no obvious casting in the above code, so the user
> may be puzzled by the ClassCastException.
> 
>> What's the best way to handle that situation?
> 
> I think the problem is that parse is not really a generic method, and
> Java does not make the target class available to the method.
> If you changed the API to add a Class parameter then you could do the
> check in the parse method.
> But that might be just as awkward to use.
> 
> It is obviously nicer to read if the compiler inserts the casts
> automatically, but the downside is that CCEs can occur when you least
> expect them.
> 
> In the sample code, it will happen immediately, but the instance could
> be stored in a collection and then the CCE might not occur until much
> later.
> 

This particular "trick" really only applies to such Object foo(...) methods where there are
two options for non-generic versions of the code:

1.  Client calls Object foo = foo(...); if (foo instanceof Foo)...
2.  Client calls Foo foo = (Foo) foo(...); potential CCE

My position is that the generic parameter RT casting trick has no effect other than to remove
the cast from option 2 above.  If a CCE were going to be raised, it would be raised regardless.
 The option 1 idiom is still perfectly valid.

Matt

> Maybe one solution would be to allow the autocasting, but provide a
> big warning in the Javadoc that it may result in CCEs at any time.
> For those who don't want to take this risk, there would need to be
> another version which guaranteed the correct class - or an Exception
> if that's not possible.
> 
>> Many thanks in advance, have a nice day!
>> Simo
>> 
>> http://people.apache.org/~simonetripodi/
>> http://www.99soft.org/
>> 
>> 
>> 
>> On Tue, Mar 1, 2011 at 4:04 PM, sebb <sebbaz@gmail.com> wrote:
>>> On 1 March 2011 08:00, Jörg Schaible <joerg.schaible@scalaris.com> wrote:
>>>> Hi Simone,
>>>> 
>>>> Simone Tripodi wrote:
>>>> 
>>>> [snip]
>>>> 
>>>> 
>>>>> @SuppressWarnings("unchecked")
>>>>> public <T> T parse(InputSource input, Class<T> returnedType)
throws
>>>>> IOException, SAXException {
>>>>>  .
>>>>>  .
>>>>>  .
>>>>>  return returnedType.cast(this.root);
>>>>> }
>>>> 
>>>> It would be nice, if we can start to avoid such method global suppressions,
>>>> because it hides possibly unwanted stuff. You can always assign the
>>>> annotation directly to a variable instead:
>>>> 
>>>> @SuppressWarnings("unchecked")
>>>> T t = returnedType.cast(this.root);
>>>> return t;
>>> 
>>> I would go a bit further and say that @SuppressWarnings should not be
>>> used unless you can prove that the cast is always valid (as may well
>>> be the case here - I've not checked), and that this should be
>>> documented in a // comment on the annotation, e.g.
>>> 
>>> @SuppressWarnings("unchecked") // OK because etc.
>>> 
>>> Otherwise, the annotation effectively gives the compiler permission to
>>> cause a ClassCastException somewhere else at some point in the future.
>>> 
>>> As with many forms of suppression, the risk is that there will be a
>>> bad reaction later ...
>>> 
>>> 
>>>> - Jörg
>>>> 
>>>> 
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>> 
>>>> 
>>> 
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>> 
>>> 
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>> 
>> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message