hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1049192
Date Tue, 14 Dec 2010 21:13:47 GMT
On 14 December 2010 20:42, Oleg Kalnichevski <olegk@apache.org> wrote:
> On Tue, 2010-12-14 at 20:18 +0000, sebb wrote:
>> On 14 December 2010 19:25, Oleg Kalnichevski <olegk@apache.org> wrote:
>> > On Tue, 2010-12-14 at 17:42 +0000, sebb@apache.org wrote:
>> >> Author: sebb
>> >> Date: Tue Dec 14 17:42:10 2010
>> >> New Revision: 1049192
>> >>
>> >> URL: http://svn.apache.org/viewvc?rev=1049192&view=rev
>> >> Log:
>> >> Add entrySet() to abstract parent class and other existing implementation
>> >>
>> >> Modified:
>> >>     httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/AbstractHttpParams.java
>> >>     httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/DefaultedHttpParams.java
>> >>
>> >
>> > Sebastian,
>> >
>> > Unfortunately, addition of an abstract method to an abstract class
>> > breaks binary compatibility
>>
>> OK, I could make it return null instead?
>>
>> > [INFO] Comparing to version: 4.1
>> > [ERROR] org.apache.http.params.AbstractHttpParams: Abstract method
>> > 'public java.util.Set entrySet()' has been added
>> >
>> >>
>> >> +    /**
>> >> +     * Provide access to the set of local parameters as Map.Entry elements.
>> >> +     * To get the entrySet for the default parameters,
>> >> +     * use {@code ((AbstractHttpParams) getDefaults()).entrySet()}
>> >> +     * @return the Set of Map.Entry<String, Object> elements
>> >> +     * @since 4.1.1
>> >> +     * @throws ClassCastException if local parameters cannot be cast
to AbstractHttpParams
>> >> +     */
>> >> +    public Set entrySet() {
>> >> +        return ((AbstractHttpParams) local).entrySet();
>> >> +    }
>> >
>> > I also do not think this cast to AbstractHttpParams is a good idea.
>>
>> Why not?
>>
>> All the provided implementations extend it.
>>
>
> You are not serious, are you? Just because all provided implementations
> in core extend it there is no guarantee there are no other
> implementations outside the library.

Agreed.

> For the love of Jesus, at least do 'instanceof' check and return null or
> some other default if the check fails.

The Javadoc makes it clear what the pre-requisites are, but yes, of
course one can check before performing the cast.

I don't think it's a good idea to return an empty set, as that is a
valid return for the successful case - how can one tell them apart?
It could make debugging more difficult that it needs to be.

I don't like returning null for Collections etc, as one has to check
the return anyway - simpler to return an Exception.

For simplicity, I went with the unchecked cast, together with the Javadoc.
A bit messy, I agree; but easy enough to fix.

> I really can't help feeling this whole idea causes more harm than good.
> There is a reason HttpParams does not provide an iterator for
> parameters.

I suppose one can create an iterable implementation of HttpParams and
use BasicHttpParams.copyParams() to copy any params to that in order
to debug the parameters, but that does seem rather long winded.

I need to give it some more thought (but I'll fix the API break meanwhile).

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

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


Mime
View raw message