Refactor method to reduce ABC Metric size #6529
Conversation
lib/jekyll/commands/serve.rb
Outdated
| jekyll_options = opts[:JekyllOptions] | ||
| ssl_cert = jekyll_options["ssl_cert"] | ||
| ssl_key = jekyll_options["ssl_key"] | ||
| return if !ssl_cert && !ssl_key |
There was a problem hiding this comment.
return unless ssl_cert || ssl_key
There was a problem hiding this comment.
the original line defined it along similar lines..
return if !opts[:JekyllOptions]["ssl_cert"] && !opts[:JekyllOptions]["ssl_key"]I just replaced with local variables..
There was a problem hiding this comment.
As long as it’s being refactored, it should be refactored to be more clear 👍
There was a problem hiding this comment.
@pathawks local testing showed me that the two are functionally different even though they appear same..
There was a problem hiding this comment.
def enable_ssl(opts)
key, cert =<
10000
/span> opts[:JekyllOptions].values_at("ssl_cert", "ssl_key")
return if cert.nil? && key.nil?
unless cert && key
raise RuntimeError, "missing --ssl_cert or " \
"--ssl_key. Both are required."
end
src = Pathutil.new(opts[:JekyllOptions]["source"])
opts[:SSLPrivateKey ] = OpenSSL::PKey::RSA.new(src.join(key).cleanpath.read)
opts[:SSLCertificate] = OpenSSL::X509::Certificate.new(src.join(cert).cleanpath.read)
opts[:SSLEnable] = true
endThere was a problem hiding this comment.
sample file:
def iflog(input="foo", alpha=false, beta=false)
return if !alpha && !beta
print "INPUT: "
puts input
end
def unlog(input="foo", alpha=false, beta=false)
return unless alpha && beta
print "INPUT: "
puts input
end- load sample in
irb - exec following couplets:
iflog # => nil unlog # => nil iflog("bar", true) # => INPUT: "bar"\n nil unlog("bar", true) # => nil iflog("bar", false, true) # => INPUT: "bar"\n nil unlog("bar", false, true) # => nil iflog("bar", true, true) # => INPUT: "bar"\n nil unlog("bar", true, true) # => INPUT: "bar"\n nil
There was a problem hiding this comment.
I don’t get it.
By De Morgan’s Law, if we want to negate the statement, we have to negate the whole thing:
if !a && !b
if !!(!a && !b)
unless !(!a && !b)
unless !!a || !!b
unless a || b
There was a problem hiding this comment.
Guys, the first one checks to see if neither exists and then leaves, the second one checks to see if either one doesn't exist and then throws. The first says if both don't exist then cool, lets bail out, the second says that if both don't exist then this is garbage.
There was a problem hiding this comment.
Writing it as so should make life much easier and avoid this debate later.
return if a.nil? && b.nil?
raise RuntimeError, "need both" unless a && b
There was a problem hiding this comment.
Who would have thought nil? would solve a semantics problem. 🤷♂️
- use parallel assignment via Hash#values_at - use nil checks to solve a semantics issue when using negated expressions with && - output better message when an exception is raised --- Though he suggested the use of Pathutil lib for both sanitizing the paths to the SSL-Certificate & SSL-Key files, and for reading them, I elected to abstract existing logic to a new private method.
|
@jekyllbot: merge +dev |
|
Thanks for your inputs on this one @envygeeks |
Assignment Branch Condition Size for the method on repo branches (as reported by RuboCop) :
master: 29.7PR-branch:20.113.64