Refactor Launcher

lib/approvals/reporters/diffmerge_reporter.rb

@@ -1,14 +1,8 @@
 module Approvals
   module Reporters
-    class DiffmergeReporter < Reporter
-      include Singleton
-
-      def self.report(received, approved)
-        instance.report(received, approved)
-      end
-
-      def default_launcher
-        Launcher.diffmerge
+    class DiffmergeReporter < NamedReporter
+      def self.command(received, approved)
+        "/Applications/DiffMerge.app/Contents/MacOS/DiffMerge --nosplash \"#{received}\" \"#{approved}\""
       end
     end
   end

lib/approvals/reporters/filelauncher_reporter.rb

@@ -1,14 +1,8 @@
 module Approvals
   module Reporters
-    class FilelauncherReporter < Reporter
-      include Singleton
-
-      def self.report(received, approved = nil)
-        instance.report(received, approved)
-      end
-
-      def default_launcher
-        Launcher.filelauncher
+    class FilelauncherReporter < NamedReporter
+      def self.command(received, _)
+        "open \"#{received}\""
       end
     end
   end

lib/approvals/reporters/launcher.rb

@@ -1,47 +1,45 @@
 module Approvals
   module Reporters
+    # TODO:  this module doesn't appear to be adding much value,
+    #        and could possibly go away?
     module Launcher
-      REPORTERS = [:opendiff, :diffmerge, :vimdiff, :tortoisediff, :filelauncher]
-
       module_function
 
+      REPORTERS = {
+        diffmerge:    DiffmergeReporter,
+        filelauncher: FilelauncherReporter,
+        opendiff:     OpendiffReporter,
+        tortoisediff: TortoisediffReporter,
+        vimdiff:      VimdiffReporter,
+      }
+
+      # Metaprogramming: create a method for each key in REPORTERS.
+      # (NB: these will become class [AKA "static"] methods due to the
+      # module_function keyword above.)
+      #
+      # Those methods return a lambda that can be invoked (with args `received,
+      # approved` to return a command.
+      #
+      # They also memoize their results (which seems unnecessary?).
+      #
+      # See usage examples in: spec/reporters/launcher_spec.rb
+      REPORTERS.each do |name, klass|
+        define_method name do # method body starts here
+          memoized(:"@#{name}") do
+            lambda {  |received, approved|
+              klass.command(received, approved)
+            }
+          end
+        end # method body ends here
+      end
+
       def memoized(instance_variable)
         unless self.instance_variable_get(instance_variable)
           value = yield
           self.instance_variable_set(instance_variable, value)
         end
         self.instance_variable_get(instance_variable)
       end
-
-      REPORTERS.each do |name|
-        define_method name do
-          memoized(:"@#{name}") do
-            lambda {|received, approved|
-              self.send("#{name}_command".to_sym, received, approved)
-            }
-          end
-        end
-      end
-
-      def opendiff_command(received, approved)
-        "opendiff #{received} #{approved}"
-      end
-
-      def diffmerge_command(received, approved)
-        "/Applications/DiffMerge.app/Contents/MacOS/DiffMerge --nosplash \"#{received}\" \"#{approved}\""
-      end
-
-      def vimdiff_command(received, approved)
-        "vimdiff #{received} #{approved}"
-      end
-
-      def tortoisediff_command(received, approved)
-        "C:\\Program Files\\TortoiseSVN\\bin\\TortoiseMerge.exe #{received} #{approved}"
-      end
-
-      def filelauncher_command(received, approved)
-        "open #{received}"
-      end
     end
   end
 end

lib/approvals/reporters/named_reporter.rb

@@ -0,0 +1,22 @@
+module Approvals
+  module Reporters
+    class NamedReporter
+      # TODO:  remove this if/when Launcher goes away
+      def default_launcher
+        Launcher.send(launcher_name)
+      end
+
+      private
+
+      def launcher_name
+        self
+          .class
+          .name
+          .split("::")
+          .last
+          .gsub(/Reporter$/, '')
+          .downcase
+      end
+    end
+  end
+end

lib/approvals/reporters/opendiff_reporter.rb

@@ -1,14 +1,8 @@
 module Approvals
   module Reporters
-    class OpendiffReporter < Reporter
-      include Singleton
-
-      def self.report(received, approved)
-        instance.report(received, approved)
-      end
-
-      def default_launcher
-        Launcher.opendiff
+    class OpendiffReporter < NamedReporter
+      def self.command(received, approved)
+        "opendiff \"#{received}\" \"#{approved}\""
       end
     end
   end

lib/approvals/reporters/tortoisediff_reporter.rb

@@ -1,14 +1,8 @@
 module Approvals
   module Reporters
-    class TortoisediffReporter < Reporter
-      include Singleton
-
-      def self.report(received, approved)
-        instance.report(received, approved)
-      end
-
-      def default_launcher
-        Launcher.tortoisediff
+    class TortoisediffReporter < NamedReporter
+      def self.command(received, approved)
+        "C:\\Program Files\\TortoiseSVN\\bin\\TortoiseMerge.exe \"#{received}\" \"#{approved}\""
       end
     end
   end

lib/approvals/reporters/vimdiff_reporter.rb

@@ -1,14 +1,8 @@
 module Approvals
   module Reporters
-    class VimdiffReporter < Reporter
-      include Singleton
-
-      def self.report(received, approved)
-        instance.report(received, approved)
-      end
-
-      def default_launcher
-        Launcher.vimdiff
+    class VimdiffReporter < NamedReporter
+      def self.command(received, approved)
+        "vimdiff \"#{received}\" \"#{approved}\""
       end
     end
   end

spec/reporters/default_launcher_spec.rb

@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+# TODO: delete these tests when Launcher goes away
+
+describe "#default_launcher" do
+  def default_launcher_for(reporter_name)
+    klass = ::Approvals::Reporters.const_get(reporter_name)
+    klass.new.default_launcher
+  end
+
+  def launcher(name)
+    Approvals::Reporters::Launcher.send(name)
+  end
+
+  it { expect( default_launcher_for(:DiffmergeReporter)    ).to eq launcher(:diffmerge) }
+  it { expect( default_launcher_for(:FilelauncherReporter) ).to eq launcher(:filelauncher) }
+  it { expect( default_launcher_for(:OpendiffReporter)     ).to eq launcher(:opendiff) }
+  it { expect( default_launcher_for(:TortoisediffReporter) ).to eq launcher(:tortoisediff) }
+  it { expect( default_launcher_for(:VimdiffReporter)      ).to eq launcher(:vimdiff) }
+end

spec/reporters/launcher_spec.rb

@@ -2,22 +2,22 @@
 
 describe Approvals::Reporters::Launcher do
   it "has a vimdiff launcher" do
-    expect(described_class.vimdiff.call('one', 'two')).to eq("vimdiff one two")
+    expect(described_class.vimdiff.call('one', 'two')).to eq('vimdiff "one" "two"')
   end
 
   it "has an opendiff launcher" do
-    expect(described_class.opendiff.call('one', 'two')).to eq("opendiff one two")
+    expect(described_class.opendiff.call('one', 'two')).to eq('opendiff "one" "two"')
   end
 
   it "has a diffmerge launcher" do
     expect(described_class.diffmerge.call('one', 'two')).to match(/DiffMerge.*\"one\"\ \"two\"/)
   end
 
   it "has a tortoisediff launcher" do
-    expect(described_class.tortoisediff.call('one', 'two')).to match(/TortoiseMerge.exe.*one.*two/)
+    expect(described_class.tortoisediff.call('one', 'two')).to match(/TortoiseMerge.exe.*\"one\".*\"two\"/)
   end
 
   it "has a filelauncher launcher" do
-    expect(described_class.filelauncher.call('one', 'two')).to eq("open one")
+    expect(described_class.filelauncher.call('one', 'two')).to eq('open "one"')
   end
 end