From 81e283b28cdd91d259e3b60687aee7ea66e9d05d Mon Sep 17 00:00:00 2001
From: Markus Roberts <Markus@reality.com>
Date: Fri, 9 Jul 2010 18:06:06 -0700
Subject: 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]
---
 lib/puppet/parameter/value_collection.rb | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

(limited to 'lib/puppet/parameter')

diff --git a/lib/puppet/parameter/value_collection.rb b/lib/puppet/parameter/value_collection.rb
index 840b892f1..eabeb30f1 100644
--- a/lib/puppet/parameter/value_collection.rb
+++ b/lib/puppet/parameter/value_collection.rb
@@ -28,9 +28,7 @@ class Puppet::Parameter::ValueCollection
                 end.join(", ") + "."
             end
 
-            unless regexes.empty?
-                @doc += "  Values can match ``" + regexes.join("``, ``") + "``."
-            end
+            @doc += "  Values can match ``" + regexes.join("``, ``") + "``." unless regexes.empty?
         end
 
         @doc
@@ -104,9 +102,7 @@ class Puppet::Parameter::ValueCollection
             value.call = options[:call] || :none
         end
 
-        if block_given? and ! value.regex?
-            value.method ||= "set_#{value.name}"
-        end
+        value.method ||= "set_#{value.name}" if block_given? and ! value.regex?
 
         value
     end
@@ -127,13 +123,9 @@ class Puppet::Parameter::ValueCollection
         unless @values.detect { |name, v| v.match?(value) }
             str = "Invalid value #{value.inspect}. "
 
-            unless values.empty?
-                str += "Valid values are #{values.join(", ")}. "
-            end
+            str += "Valid values are #{values.join(", ")}. " unless values.empty?
 
-            unless regexes.empty?
-                str += "Valid values match #{regexes.join(", ")}."
-            end
+            str += "Valid values match #{regexes.join(", ")}." unless regexes.empty?
 
             raise ArgumentError, str
         end
-- 
cgit