(#5079) Refactor and cleanup mcxcontent provider

This is a rework for diff clarity of a patch submitted by
Sandor Szuecs <sandor.szuecs@fu-berlin.de>

Reviewed-by: Paul Berry <paul@puppetlabs.com>

1 files changed, 23 insertions, 40 deletions

diff --git a/lib/puppet/provider/mcx/mcxcontent.rb b/lib/puppet/provider/mcx/mcxcontent.rb
index b7cc40b87..3ad437b53 100644
--- a/lib/puppet/provider/mcx/mcxcontent.rb
+++ b/lib/puppet/provider/mcx/mcxcontent.rb
@@ -53,29 +53,25 @@ Puppet::Type.type(:mcx).provide :mcxcontent, :parent => Puppet::Provider do
   confine :operatingsystem => :darwin
   defaultfor :operatingsystem => :darwin
 
-  # self.instances is all important.
-  # This is the only class method, it returns
-  # an array of instances of this class.
   def self.instances
     mcx_list = []
-    for ds_type in TypeMap.keys
+    TypeMap.keys.each do |ds_type|
       ds_path = "/Local/Default/#{TypeMap[ds_type]}"
       output = dscl 'localhost', '-list', ds_path
       member_list = output.split
-      for ds_name in member_list
+      member_list.each do |ds_name|
         content = mcxexport(ds_type, ds_name)
         if content.empty?
           Puppet.debug "/#{TypeMap[ds_type]}/#{ds_name} has no MCX data."
         else
           # This node has MCX data.
 
-            rsrc = self.new(
-              :name => "/#{TypeMap[ds_type]}/#{ds_name}",
-                :ds_type => ds_type,
-                :ds_name => ds_name,
-
-                :content => content)
-          mcx_list << rsrc
+          mcx_list << self.new(
+            :name => "/#{TypeMap[ds_type]}/#{ds_name}",
+            :ds_type => ds_type,
+            :ds_name => ds_name,
+            :content => content
+          )
         end
       end
     end
@@ -104,42 +100,34 @@ Puppet::Type.type(:mcx).provide :mcxcontent, :parent => Puppet::Provider do
   end
 
   def exists?
-    # JJM Just re-use the content method and see if it's empty.
     begin
-      mcx = content
+      has_mcx?
     rescue Puppet::ExecutionFailure => e
       return false
     end
-    has_mcx = ! mcx.empty?
   end
 
   def content
     ds_parms = get_dsparams
 
-      mcx = self.class.mcxexport(
-        ds_parms[:ds_type],
-
-          ds_parms[:ds_name])
-    mcx
+    self.class.mcxexport(ds_parms[:ds_type], ds_parms[:ds_name])
   end
 
   def content=(value)
     # dscl localhost -mcximport
     ds_parms = get_dsparams
 
-      mcx = mcximport(
-        ds_parms[:ds_type],
-          ds_parms[:ds_name],
-
-          resource[:content])
-    mcx
+    mcximport(ds_parms[:ds_type], ds_parms[:ds_name], resource[:content])
   end
 
   private
 
+  def has_mcx?
+    !content.empty?
+  end
+
   def mcximport(ds_type, ds_name, val)
     ds_t = TypeMap[ds_type]
-    ds_n = ds_name.to_s
     ds_path = "/Local/Default/#{ds_t}/#{ds_name}"
 
     tmp = Tempfile.new('puppet_mcx')
@@ -155,33 +143,31 @@ Puppet::Type.type(:mcx).provide :mcxcontent, :parent => Puppet::Provider do
 
   # Given the resource name string, parse ds_type out.
   def parse_type(name)
-    tmp = name.split('/')[1]
-    if ! tmp.is_a? String
+    ds_type = name.split('/')[1]
+    unless ds_type
       raise MCXContentProviderException,
       "Coult not parse ds_type from resource name '#{name}'.  Specify with ds_type parameter."
     end
     # De-pluralize and downcase.
-    tmp = tmp.chop.downcase.to_sym
-    if not TypeMap.keys.member? tmp
+    ds_type = ds_type.chop.downcase.to_sym
+    unless TypeMap.key? ds_type
       raise MCXContentProviderException,
       "Coult not parse ds_type from resource name '#{name}'.  Specify with ds_type parameter."
     end
-    tmp
+    ds_type
   end
 
   # Given the resource name string, parse ds_name out.
   def parse_name(name)
     ds_name = name.split('/')[2]
-    if ! ds_name.is_a? String
+    unless ds_name
       raise MCXContentProviderException,
       "Could not parse ds_name from resource name '#{name}'.  Specify with ds_name parameter."
     end
     ds_name
   end
 
-  # Gather ds_type and ds_name from resource or
-  # parse it out of the name.
-  # This is a private instance method, not a class method.
+  # Gather ds_type and ds_name from resource or parse it out of the name.
   def get_dsparams
     ds_type = resource[:ds_type]
     ds_type ||= parse_type(resource[:name])
@@ -190,13 +176,10 @@ Puppet::Type.type(:mcx).provide :mcxcontent, :parent => Puppet::Provider do
     ds_name = resource[:ds_name]
     ds_name ||= parse_name(resource[:name])
 
-    rval = {
+    {
       :ds_type => ds_type.to_sym,
       :ds_name => ds_name,
     }
-
-    return rval
-
   end
 
 end