hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: svn commit: r1049192
Date Tue, 14 Dec 2010 21:37:01 GMT
On Tue, 2010-12-14 at 21:13 +0000, sebb wrote:

...

> >> >> +    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).
> 

HttpParams and friends turned out to be the worst part of the HttpCore
API. I wish we had had more collective wisdom to have done it
differently and better. We can't change it now, but let us not make it
even worse. 

I think optional / extended interface is marginally less ugly than a
downcast to a concrete implementation. Besides, it provides an easier
transition to a cleaner API in the next major release. In 4.x users can
be advised to always implement both interfaces. In 5.0 methods from the
extended interface can be moved to the base interface without causing
any code breakage at all as long as concrete classes implement both
interfaces. 

Oleg



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


Mime
View raw message