You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by se...@apache.org on 2010/12/14 18:42:11 UTC
svn commit: r1049192 - in
/httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params:
AbstractHttpParams.java DefaultedHttpParams.java
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
Modified: httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/AbstractHttpParams.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/AbstractHttpParams.java?rev=1049192&r1=1049191&r2=1049192&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/AbstractHttpParams.java (original)
+++ httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/AbstractHttpParams.java Tue Dec 14 17:42:10 2010
@@ -27,6 +27,8 @@
package org.apache.http.params;
+import java.util.Set;
+
import org.apache.http.params.HttpParams;
@@ -106,4 +108,13 @@ public abstract class AbstractHttpParams
return !getBooleanParameter(name, false);
}
+
+ /**
+ * Provide access to the set of parameters as Map.Entry elements.
+ *
+ * @return the Set of Map.Entry<String, Object> elements
+ * @since 4.1.1
+ */
+ public abstract Set entrySet();
+
} // class AbstractHttpParams
Modified: httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/DefaultedHttpParams.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/DefaultedHttpParams.java?rev=1049192&r1=1049191&r2=1049192&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/DefaultedHttpParams.java (original)
+++ httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/params/DefaultedHttpParams.java Tue Dec 14 17:42:10 2010
@@ -27,6 +27,8 @@
package org.apache.http.params;
+import java.util.Set;
+
import org.apache.http.params.HttpParams;
/**
@@ -42,6 +44,12 @@ public final class DefaultedHttpParams e
private final HttpParams local;
private final HttpParams defaults;
+ /**
+ * Create the defaulted set of HttpParams.
+ *
+ * @param local the mutable set of HttpParams
+ * @param defaults the default set of HttpParams, not mutated by this class
+ */
public DefaultedHttpParams(final HttpParams local, final HttpParams defaults) {
super();
if (local == null) {
@@ -90,8 +98,24 @@ public final class DefaultedHttpParams e
return this.local.setParameter(name, value);
}
+ /**
+ *
+ * @return the default HttpParams collection
+ */
public HttpParams getDefaults() {
return this.defaults;
}
+ /**
+ * 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();
+ }
+
}
Re: svn commit: r1049192
Posted by Oleg Kalnichevski <ol...@apache.org>.
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
Re: svn commit: r1049192
Posted by sebb <se...@gmail.com>.
On 14 December 2010 20:42, Oleg Kalnichevski <ol...@apache.org> wrote:
> On Tue, 2010-12-14 at 20:18 +0000, sebb wrote:
>> On 14 December 2010 19:25, Oleg Kalnichevski <ol...@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
Re: svn commit: r1049192
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2010-12-14 at 20:18 +0000, sebb wrote:
> On 14 December 2010 19:25, Oleg Kalnichevski <ol...@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.
For the love of Jesus, at least do 'instanceof' check and return null or
some other default if the check fails.
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.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1049192
Posted by sebb <se...@gmail.com>.
On 14 December 2010 19:25, Oleg Kalnichevski <ol...@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.
> 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
Re: svn commit: r1049192
Posted by Oleg Kalnichevski <ol...@apache.org>.
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
[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.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org