From 012c7768803c7d1ddc5e3e91bbf5c038e975b653 Mon Sep 17 00:00:00 2001
From: Thibault Jouan <tj@a13.fr>
Date: Sat, 21 Dec 2013 03:08:01 +0000
Subject: [PATCH] Simplify Remote::Environment with factory method:

* Implement .new_from_string factory class method;
* Rename private method #parse_from_string as class method
  .string_to_hash;
* Remove argument kind detection logic in constructor.
---
 lib/producer/core/remote.rb                   |  2 +-
 lib/producer/core/remote/environment.rb       | 23 +++++++-------
 spec/producer/core/remote/environment_spec.rb | 30 ++++++++++++-------
 spec/producer/core/remote_spec.rb             |  4 +--
 4 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/lib/producer/core/remote.rb b/lib/producer/core/remote.rb
index e9db46a..12f77a4 100644
--- a/lib/producer/core/remote.rb
+++ b/lib/producer/core/remote.rb
@@ -45,7 +45,7 @@ module Producer
       end
 
       def environment
-        Environment.new(execute 'env')
+        Environment.new_from_string(execute 'env')
       end
     end
   end
diff --git a/lib/producer/core/remote/environment.rb b/lib/producer/core/remote/environment.rb
index f161ecb..572924a 100644
--- a/lib/producer/core/remote/environment.rb
+++ b/lib/producer/core/remote/environment.rb
@@ -2,24 +2,23 @@ module Producer
   module Core
     class Remote
       class Environment
+        class << self
+          def string_to_hash(str)
+            Hash[str.each_line.map { |l| l.chomp.split '=', 2 }]
+          end
+
+          def new_from_string(str)
+            new string_to_hash str
+          end
+        end
+
         require 'forwardable'
 
         extend Forwardable
         def_delegator :@variables, :has_key?
 
         def initialize(variables)
-          case variables
-          when String
-            @variables = parse_from_string variables
-          else
-            @variables = variables
-          end
-        end
-
-        private
-
-        def parse_from_string(str)
-          Hash[str.each_line.map { |l| l.chomp.split '=', 2 }]
+          @variables = variables
         end
       end
     end
diff --git a/spec/producer/core/remote/environment_spec.rb b/spec/producer/core/remote/environment_spec.rb
index 78998cf..4337dd4 100644
--- a/spec/producer/core/remote/environment_spec.rb
+++ b/spec/producer/core/remote/environment_spec.rb
@@ -7,19 +7,29 @@ module Producer::Core
     let(:argument)        { variables }
     subject(:environment) { Remote::Environment.new(argument) }
 
-    describe '#initialize' do
-      context 'when a hash is given' do
-        it 'assigns the key/value pairs' do
-          expect(environment.instance_eval { @variables }).to eq variables
-        end
+    describe '.string_to_hash' do
+      it 'converts key=value pairs separated by new lines to a hash' do
+        expect(Remote::Environment.string_to_hash(string))
+          .to eq variables
+      end
+    end
+
+    describe '.new_from_string' do
+      it 'builds a new instance after converting from string' do
+        expect(Remote::Environment).to receive(:new).with(variables)
+        Remote::Environment.new_from_string(string)
       end
 
-      context 'when a string is given' do
-        let(:argument) { string }
+      it 'returns the instance' do
+        environment = double 'environment'
+        allow(Remote::Environment).to receive(:new) { environment }
+        expect(Remote::Environment.new_from_string(string)).to be environment
+      end
+    end
 
-        it 'assigns the key/value pairs' do
-          expect(environment.instance_eval { @variables }).to eq variables
-        end
+    describe '#initialize' do
+      it 'assigns the key/value pairs' do
+        expect(environment.instance_eval { @variables }).to eq variables
       end
     end
 
diff --git a/spec/producer/core/remote_spec.rb b/spec/producer/core/remote_spec.rb
index 39b88eb..a95c7ce 100644
--- a/spec/producer/core/remote_spec.rb
+++ b/spec/producer/core/remote_spec.rb
@@ -128,13 +128,13 @@ module Producer::Core
       end
 
       it 'builds a remote environment with the result of `env` command' do
-        expect(Remote::Environment).to receive(:new).with(output)
+        expect(Remote::Environment).to receive(:new_from_string).with(output)
         remote.environment
       end
 
       it 'returns the environment' do
         environment = double 'environment'
-        allow(Remote::Environment).to receive(:new) { environment }
+        allow(Remote::Environment).to receive(:new_from_string) { environment }
         expect(remote.environment).to be environment
       end
     end