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

12 files changed, 17 insertions, 70 deletions

diff --git a/lib/puppet/util/autoload/file_cache.rb b/lib/puppet/util/autoload/file_cache.rb
index 873dc8fb7..44d988969 100644
--- a/lib/puppet/util/autoload/file_cache.rb
+++ b/lib/puppet/util/autoload/file_cache.rb
@@ -48,8 +48,7 @@ module Puppet::Util::Autoload::FileCache
 
     def found_file?(path, type = nil)
         if data = found_files[path] and ! data_expired?(data[:time])
-            return false if type and ! data[:stat].send(type)
-            return true
+            return(type and ! data[:stat].send(type)) ? false : true
         else
             return false
         end
@@ -60,11 +59,7 @@ module Puppet::Util::Autoload::FileCache
     end
 
     def missing_file?(path)
-        if time = missing_files[path] and ! data_expired?(time)
-            return true
-        else
-            return false
-        end
+        return !!(time = missing_files[path] and ! data_expired?(time))
     end
 
     def missing_file(path)
diff --git a/lib/puppet/util/backups.rb b/lib/puppet/util/backups.rb
index e08bf57de..1e051498c 100644
--- a/lib/puppet/util/backups.rb
+++ b/lib/puppet/util/backups.rb
@@ -12,8 +12,7 @@ module Puppet::Util::Backups
         file ||= self[:path]
         return true unless FileTest.exists?(file)
 
-        return perform_backup_with_bucket(file) if self.bucket
-        return perform_backup_with_backuplocal(file, self[:backup])
+        return(self.bucket ? perform_backup_with_bucket(file) : perform_backup_with_backuplocal(file, self[:backup]))
     end
 
     private
diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/checksums.rb
index 41f878687..7b3ef6e15 100644
--- a/lib/puppet/util/checksums.rb
+++ b/lib/puppet/util/checksums.rb
@@ -8,20 +8,12 @@ module Puppet::Util::Checksums
 
     # Strip the checksum type from an existing checksum
     def sumdata(checksum)
-        if checksum =~ /^\{(\w+)\}(.+)/
-            return $2
-        else
-            return nil
-        end
+        return checksum =~ /^\{(\w+)\}(.+)/ ? $2 : nil
     end
 
     # Strip the checksum type from an existing checksum
     def sumtype(checksum)
-        if checksum =~ /^\{(\w+)\}/
-            return $1
-        else
-            return nil
-        end
+        return checksum =~ /^\{(\w+)\}/ ? $1 : nil
     end
 
     # Calculate a checksum using Digest::MD5.
diff --git a/lib/puppet/util/feature.rb b/lib/puppet/util/feature.rb
index 30422ce4c..8e2f81fa4 100644
--- a/lib/puppet/util/feature.rb
+++ b/lib/puppet/util/feature.rb
@@ -48,11 +48,7 @@ class Puppet::Util::Feature
         feature = method.to_s.sub(/\?$/, '')
         @loader.load(feature)
 
-        if respond_to?(method)
-            return self.send(method)
-        else
-            return false
-        end
+        return respond_to?(method) && self.send(method)
     end
 
     # Actually test whether the feature is present.  We only want to test when
diff --git a/lib/puppet/util/fileparsing.rb b/lib/puppet/util/fileparsing.rb
index 7348b8946..c8ab53d6a 100644
--- a/lib/puppet/util/fileparsing.rb
+++ b/lib/puppet/util/fileparsing.rb
@@ -137,11 +137,7 @@ module Puppet::Util::FileParsing
 
     # Try to match a specific text line.
     def handle_text_line(line, record)
-        if line =~ record.match
-            return {:record_type => record.name, :line => line}
-        else
-            return nil
-        end
+        return line =~ record.match ? {:record_type => record.name, :line => line} : nil
     end
 
     # Try to match a record.
diff --git a/lib/puppet/util/inifile.rb b/lib/puppet/util/inifile.rb
index b90134835..3488ce54d 100644
--- a/lib/puppet/util/inifile.rb
+++ b/lib/puppet/util/inifile.rb
@@ -56,8 +56,7 @@ module Puppet::Util::IniConfig
         # exists, return nil
         def [](key)
             entry = find_entry(key)
-            return nil if entry.nil?
-            return entry[1]
+            return(entry.nil? ? nil : entry[1])
         end
 
         # Format the section as text in the way it should be
diff --git a/lib/puppet/util/ldap/manager.rb b/lib/puppet/util/ldap/manager.rb
index cfa7cb3f7..93e8baac3 100644
--- a/lib/puppet/util/ldap/manager.rb
+++ b/lib/puppet/util/ldap/manager.rb
@@ -103,8 +103,7 @@ class Puppet::Util::Ldap::Manager
 
     # Create our normal search filter.
     def filter
-        return "objectclass=#{objectclasses[0]}" if objectclasses.length == 1
-        return "(&(objectclass=" + objectclasses.join(")(objectclass=") + "))"
+        return(objectclasses.length == 1 ? "objectclass=#{objectclasses[0]}" : "(&(objectclass=" + objectclasses.join(")(objectclass=") + "))")
     end
 
     # Find the associated entry for a resource.  Returns a hash, minus
@@ -209,8 +208,7 @@ class Puppet::Util::Ldap::Manager
                 result << entry2provider(entry)
             end
         end
-        return nil if result.empty?
-        return result
+        return(result.empty? ? nil : result)
     end
 
     # Update the ldap entry with the desired state.
diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index 5dea6d67f..16a4eb239 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -71,11 +71,7 @@ class Puppet::Util::Log
     def Log.create(hash)
         raise Puppet::DevError, "Logs require a level" unless hash.include?(:level)
         raise Puppet::DevError, "Invalid log level #{hash[:level]}" unless @levels.index(hash[:level])
-        if @levels.index(hash[:level]) >= @loglevel
-            return Puppet::Util::Log.new(hash)
-        else
-            return nil
-        end
+        return @levels.index(hash[:level]) >= @loglevel ? Puppet::Util::Log.new(hash) : nil
     end
 
     def Log.destinations
diff --git a/lib/puppet/util/provider_features.rb b/lib/puppet/util/provider_features.rb
index d106d473c..37f11aa12 100644
--- a/lib/puppet/util/provider_features.rb
+++ b/lib/puppet/util/provider_features.rb
@@ -15,11 +15,7 @@ module Puppet::Util::ProviderFeatures
         # Are all of the requirements met?
         def available?(obj)
             if self.methods
-                if methods_available?(obj)
-                    return true
-                else
-                    return false
-                end
+                return !!methods_available?(obj)
             else
                 # In this case, the provider has to declare support for this
                 # feature, and that's been checked before we ever get to the
@@ -116,11 +112,7 @@ module Puppet::Util::ProviderFeatures
             # determine if the feature is present.
             @feature_module.send(:define_method, :feature?) do |name|
                 method = name.to_s + "?"
-                if respond_to?(method) and send(method)
-                    return true
-                else
-                    return false
-                end
+                return !!(respond_to?(method) and send(method))
             end
 
             # Create a method that will list all functional features.
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index b8f4effba..055492c84 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -54,11 +54,7 @@ class Puppet::Util::Settings
     # Is our parameter a boolean parameter?
     def boolean?(param)
         param = param.to_sym
-        if @config.include?(param) and @config[param].kind_of? BooleanSetting
-            return true
-        else
-            return false
-        end
+        return !!(@config.include?(param) and @config[param].kind_of? BooleanSetting)
     end
 
     # Remove all set values, potentially skipping cli values.
diff --git a/lib/puppet/util/settings/setting.rb b/lib/puppet/util/settings/setting.rb
index c3a037102..848c7780e 100644
--- a/lib/puppet/util/settings/setting.rb
+++ b/lib/puppet/util/settings/setting.rb
@@ -50,19 +50,11 @@ class Puppet::Util::Settings::Setting
     end
 
     def iscreated?
-        if defined?(@iscreated)
-            return @iscreated
-        else
-            return false
-        end
+        return defined?(@iscreated) && @iscreated
     end
 
     def set?
-        if defined?(@value) and ! @value.nil?
-            return true
-        else
-            return false
-        end
+        return !!(defined?(@value) and ! @value.nil?)
     end
 
     # short name for the celement
diff --git a/lib/puppet/util/subclass_loader.rb b/lib/puppet/util/subclass_loader.rb
index 505c4b0a5..02d4af159 100644
--- a/lib/puppet/util/subclass_loader.rb
+++ b/lib/puppet/util/subclass_loader.rb
@@ -63,11 +63,7 @@ module Puppet::Util::SubclassLoader
             super
         end
         return nil unless defined?(@subclassname)
-        if c = self.send(@subclassname, method)
-            return c
-        else
-            return nil
-        end
+        return self.send(@subclassname, method) || nil
     end
 
     # Retrieve or calculate a name.