Hi, I agree with Roland about #1. A renaming of the method makes sense there. I like the sound of "checkHostname()".
One note, though: our HostnameVerifier interface *does* subclass javax.net.ssl.HostnameVerifier, so that fact probably alleviates your concerns somewhat as well. People can use the boolean method provided if they really want to. I'll rename the methods as part of HTTPCLIENT-617 when I get around to that patch. (Is it okay to address style concerns like this in an existing bug, or should a new bug be created?). Thanks, Roland, for taking a look, and commenting! I was pleased to see the JUnit tests I created catching you (so soon after writing them!)! :-) I looked at your patch for HTTPCLIENT-475 and I thought it looked really good. yours, Julius 1) HostnameVerifier
We have an interface with that name, and standard Java has one too in javax.net.ssl. Both define methods called verify() that check whether a hostname is acceptable with respect to presented certificates and such. Although they all share the same name, the standard Java method returns a boolean, while ours throw an exception if the verification fails. This is BAD. As in REALLY bad. Somebody gets used to the exception being thrown, then someday calls the standard Java method without noticing the difference, and a verification failure will just go unnoticed! Now I don't mind throwing an exception instead of returning a boolean. It allows for an error message that tells about the exact reason for the verification failure. But giving these methods the same name as the original method with a completely different usage pattern is not good. We must find a different name to point out the different usage pattern. Maybe check(...) or assert(...) or whatever, but not verify(...).
-- yours, Julius Davies 416-652-0183 http://juliusdavies.ca/
