Raises errors when giving encryption as non-Hash object #257
Conversation
|
👍 it would be nice to validate all given parameters from constructor. |
| @@ -540,6 +540,9 @@ def initialize(args = {}) | |||
| @base = args[:base] || DefaultTreebase | |||
| @force_no_page = args[:force_no_page] || DefaultForceNoPage | |||
| @encryption = args[:encryption] # may be nil | |||
| if !@encryption.nil? and !@encryption.is_a?(Hash) | |||
There was a problem hiding this comment.
Can we switch this to @encryption && !@encryption.is_a?(Hash)? This makes the nil check implicit, and the && operator is higher precedence than and (doesn't matter in this case, but I prefer it when used in a conditional)
There was a problem hiding this comment.
These operators still hurt my brain, if we replace the and operators in the this codebase to && all the tests fail.
http://devblog.avdi.org/2014/08/26/how-to-use-rubys-english-andor-operators-without-going-nuts/
There was a problem hiding this comment.
Alternatively, we could also use a single type check with @encryption.respond_to?(:[]). I don't have a strong preference here.
@mynameisrufus why would && fail here?
There was a problem hiding this comment.
Not here specifically, but when trying to replace and with && throughout the entire codebase (I tried it) all the test fail :)
There was a problem hiding this comment.
Thanks for clarifying. I wasn't trying to make so bold a claim ;)
There was a problem hiding this comment.
@jch @mynameisrufus Thank you for pointing out this condition. made my understanding about these operators clear 👍
In #250 ,
Net::LDAPof the latest version 0.13.0 accepts encryption options on its initializer. It causes an error when creating a connection as reported.This PR provides a validation which check whether a given
encryptionoption is a Hash or not.