Code smell: Line modifiers are preferred to one-line blocks.

* Replaced 6 occurances of (while .*?) *do$ with

  The do is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          while line = f.gets do
      becomes:
          while line = f.gets
      The code:
          while line = shadow.gets do
      becomes:
          while line = shadow.gets
      The code:
          while wrapper = zeros.pop do
      becomes:
          while wrapper = zeros.pop

* Replaced 19 occurances of ((if|unless) .*?) *then$ with

  The then is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } then
      becomes:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
      The code:
          unless defined?(@spec_command) then
      becomes:
          unless defined?(@spec_command)
      The code:
          if c == ?\n then
      becomes:
          if c == ?\n

* Replaced 758 occurances of

      ((?:if|unless|while|until) .*)
          (.*)
      end

  with

  The one-line form is preferable provided:

      * The condition is not used to assign a variable
      * The body line is not already modified
      * The resulting line is not too long

  3 Examples:

      The code:
          if Puppet.features.libshadow?
              has_feature :manages_passwords
          end
      becomes:
          has_feature :manages_passwords if Puppet.features.libshadow?
      The code:
          unless (defined?(@current_pool) and @current_pool)
              @current_pool = process_zpool_data(get_pool_data)
          end
      becomes:
          @current_pool = process_zpool_data(get_pool_data) unless (defined?(@current_pool) and @current_pool)
      The code:
          if Puppet[:trace]
              puts detail.backtrace
          end
      becomes:
          puts detail.backtrace if Puppet[:trace]

2 files changed, 9 insertions, 27 deletions

diff --git a/lib/puppet/property/keyvalue.rb b/lib/puppet/property/keyvalue.rb
index df6560643..a1495b6a7 100644
--- a/lib/puppet/property/keyvalue.rb
+++ b/lib/puppet/property/keyvalue.rb
@@ -45,16 +45,12 @@ module Puppet
                 return {} if current == :absent
 
                 #inclusive means we are managing everything so if it isn't in should, its gone
-                if inclusive?
-                    current.each_key { |key| current[key] = nil }
-                end
+                current.each_key { |key| current[key] = nil } if inclusive?
                 current
             end
 
             def should
-                unless defined?(@should) and @should
-                    return nil
-                end
+                return nil unless defined?(@should) and @should
 
                 members = hashify(@should)
                 current = process_current_hash(retrieve)
@@ -81,13 +77,9 @@ module Puppet
             end
 
             def insync?(is)
-                unless defined?(@should) and @should
-                    return true
-                end
+                return true unless defined?(@should) and @should
 
-                unless is
-                    return true
-                end
+                return true unless is
 
                 return (is == self.should)
             end
diff --git a/lib/puppet/property/list.rb b/lib/puppet/property/list.rb
index 2e38aeccc..2254ed249 100644
--- a/lib/puppet/property/list.rb
+++ b/lib/puppet/property/list.rb
@@ -22,9 +22,7 @@ module Puppet
             end
 
             def add_should_with_current(should, current)
-                if current.is_a?(Array)
-                    should += current
-                end
+                should += current if current.is_a?(Array)
                 should.uniq
             end
 
@@ -38,15 +36,11 @@ module Puppet
             end
 
             def should
-                unless defined?(@should) and @should
-                    return nil
-                end
+                return nil unless defined?(@should) and @should
 
                 members = @should
                 #inclusive means we are managing everything so if it isn't in should, its gone
-                if ! inclusive?
-                    members = add_should_with_current(members, retrieve)
-                end
+                members = add_should_with_current(members, retrieve) if ! inclusive?
 
                 dearrayify(members)
             end
@@ -72,13 +66,9 @@ module Puppet
             end
 
             def insync?(is)
-                unless defined?(@should) and @should
-                    return true
-                end
+                return true unless defined?(@should) and @should
 
-                unless is
-                    return true
-                end
+                return true unless is
 
                 return (prepare_is_for_comparison(is) == self.should)
             end