From 889158ad57e33df083613d6f7d136b2e11aaa16a Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Fri, 9 Jul 2010 18:06:12 -0700 Subject: Code smell: Booleans are first class values. * Replaced 2 occurances of def (.*) begin (.*) = Integer\((.*)\) return \2 rescue ArgumentError \2 = nil end if \2 = (.*) return \2 else return false end end with 2 Examples: The code: def validuser?(value) begin number = Integer(value) return number rescue ArgumentError number = nil end if number = uid(value) return number else return false end end becomes: def validuser?(value) Integer(value) rescue uid(value) || false end The code: def validgroup?(value) begin number = Integer(value) return number rescue ArgumentError number = nil end if number = gid(value) return number else return false end end becomes: def validgroup?(value) Integer(value) rescue gid(value) || false end * Replaced 28 occurances of return (.*?) if (.*) return (.*) with 3 Examples: The code: return send(options[:mode]) if [:rdoc, :trac, :markdown].include?(options[:mode]) return other becomes: return[:rdoc, :trac, :markdown].include?(options[:mode]) ? send(options[:mode]) : other The code: return true if known_resource_types.definition(name) return false becomes: return(known_resource_types.definition(name) ? true : false) The code: return :rest if request.protocol == 'https' return Puppet::FileBucket::File.indirection.terminus_class becomes: return(request.protocol == 'https' ? :rest : Puppet::FileBucket::File.indirection.terminus_class) * Replaced no occurances of return (.*?) unless (.*) return (.*) with * Replaced 7 occurances of if (.*) (.*[^:])false else \2true end with 3 Examples: The code: if RUBY_PLATFORM == "i386-mswin32" InstallOptions.ri = false else InstallOptions.ri = true end becomes: InstallOptions.ri = RUBY_PLATFORM != "i386-mswin32" The code: if options[:references].length > 1 with_contents = false else with_contents = true end becomes: with_contents = options[:references].length <= 1 The code: if value == false or value == "" or value == :undef return false else return true end becomes: return (value != false and value != "" and value != :undef) * Replaced 19 occurances of if (.*) (.*[^:])true else \2false end with 3 Examples: The code: if Puppet::Util::Log.level == :debug return true else return false end becomes: return Puppet::Util::Log.level == :debug The code: if satisfies?(*features) return true else return false end becomes: return !!satisfies?(*features) The code: if self.class.parsed_auth_db.has_key?(resource[:name]) return true else return false end becomes: return !!self.class.parsed_auth_db.has_key?(resource[:name]) * Replaced 1 occurance of if ([a-z_]) = (.*) (.*[^:])\1 else \3(.*) end with 1 Example: The code: if c = self.send(@subclassname, method) return c else return nil end becomes: return self.send(@subclassname, method) || nil * Replaced 2 occurances of if (.*) (.*[^:])\1 else \2false end with 2 Examples: The code: if hash[:Local] @local = hash[:Local] else @local = false end becomes: @local = hash[:Local] The code: if hash[:Local] @local = hash[:Local] else @local = false end becomes: @local = hash[:Local] * Replaced 10 occurances of if (.*) (.*[^:])(.*) else \2false end with 3 Examples: The code: if defined?(@isnamevar) return @isnamevar else return false end becomes: return defined?(@isnamevar) && @isnamevar The code: if defined?(@required) return @required else return false end becomes: return defined?(@required) && @required The code: if number = uid(value) return number else return false end becomes: return (number = uid(value)) && number * Replaced no occurances of if (.*) (.*[^:])nil else \2(true) end with * Replaced no occurances of if (.*) (.*[^:])true else \2nil end with * Replaced no occurances of if (.*) (.*[^:])\1 else \2nil end with * Replaced 23 occurances of if (.*) (.*[^:])(.*) else \2nil end with 3 Examples: The code: if node = Puppet::Node.find(hostname) env = node.environment else env = nil end becomes: env = (node = Puppet::Node.find(hostname)) ? node.environment : nil The code: if mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files? return @mounts[MODULES].copy(mod.name, mod.file_directory) else return nil end becomes: return (mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?) ? @mounts[MODULES].copy(mod.name, mod.file_directory) : nil The code: if hash.include?(:CA) and hash[:CA] @ca = Puppet::SSLCertificates::CA.new() else @ca = nil end becomes: @ca = (hash.include?(:CA) and hash[:CA]) ? Puppet::SSLCertificates::CA.new() : nil --- lib/puppet/network/handler/fileserver.rb | 18 +++--------------- lib/puppet/network/handler/master.rb | 12 ++---------- 2 files changed, 5 insertions(+), 25 deletions(-) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb index cb54ca4e4..b7a0c1387 100755 --- a/lib/puppet/network/handler/fileserver.rb +++ b/lib/puppet/network/handler/fileserver.rb @@ -92,11 +92,7 @@ class Puppet::Network::Handler @mounts = {} @files = {} - if hash[:Local] - @local = hash[:Local] - else - @local = false - end + @local = hash[:Local] @noreadconfig = true if hash[:Config] == false @@ -236,18 +232,10 @@ class Puppet::Network::Handler unless hostname = (client || Facter.value("hostname")) raise ArgumentError, "Could not find hostname" end - if node = Puppet::Node.find(hostname) - env = node.environment - else - env = nil - end + env = (node = Puppet::Node.find(hostname)) ? node.environment : nil # And use the environment to look up the module. - if mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files? - return @mounts[MODULES].copy(mod.name, mod.file_directory) - else - return nil - end + return (mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?) ? @mounts[MODULES].copy(mod.name, mod.file_directory) : nil end # Read the configuration file. diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb index d55046b5b..690e7079e 100644 --- a/lib/puppet/network/handler/master.rb +++ b/lib/puppet/network/handler/master.rb @@ -29,19 +29,11 @@ class Puppet::Network::Handler def initialize(hash = {}) args = {} - if hash[:Local] - @local = hash[:Local] - else - @local = false - end + @local = hash[:Local] args[:Local] = true - if hash.include?(:CA) and hash[:CA] - @ca = Puppet::SSLCertificates::CA.new() - else - @ca = nil - end + @ca = (hash.include?(:CA) and hash[:CA]) ? Puppet::SSLCertificates::CA.new() : nil # This is only used by the cfengine module, or if --loadclasses was # specified in +puppet+. -- cgit