hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Becke <be...@u.washington.edu>
Subject Re: [PATCH]Protocol take 1
Date Sat, 14 Dec 2002 00:19:44 GMT
> Protocol - should have a toString() method.  There is at least one 
> place where debug logging wants to output a "protocol" object, and 
> doesn't call protocol.getScheme().  If you have a toString() method, 
> this wouldn't be a problem - or you could change the debug logging 
> statement....

Can do.

> Protocol.getPort() - perhaps getDefaultPort()?


> HttpConnection.setSecure() - I haven't thought this through fully, but 
> might the request for "setSecure()" be an operation on the protocol as 
> well - in the sense that "http" knows its secure counterpart "https", 
> and vice-versa?

Not sure about this one.  I think the protocols should be static.  My 
intention here was to keep backwards compatibility more than anything 
else though.  What makes the most sense to everyone else?

> HttpConnection.open() - does it make sense to have a Protocol provide 
> its socket factory, which would further simplify this function?

That's what's happening now I think.

> HttpMethodBase.addHostRequestHeader() - has "if (conn.isSecure)" logic 
> at the end that could be replaced by a new function on the protocol.

Good catch, missed that one.

> There are three places I think where you the same logic appears: if 
> port <= 0 ? protocol.getPort() : port.  Could that logic be moved to a 
> central place?

I'll take a look.

> I also like the idea of "Protocol" being an interface, rather than a 
> concrete class, and then add a default implementation of that 
> interface that looks like the class.  I'm also interface happy, as one 
> of my previous emails indicated.

I agree having an interface might be nice.  I'm not sure it's warranted 
in this case though.  The Protocol class doesn't really do much other 
than store some values.  Can you think of a use case for making it an 

> More Javadoc particularly on the Protocol and ProtocolManager classes 
> themselves!

Yes, definitely needs more docs.

Thank you for the thoughtful comments.


View raw message