Pre-determination of static diff attributes via attr_reader

Author: Kevin Paulisse

lib/octocatalog-diff/api/v1/diff.rb

@@ -17,7 +17,7 @@ module V1
       # easier to deal with. We recommend using the named options, rather than #raw or the indexed array,
       # as the raw object and indexed array are not guaranteed to be stable.
       class Diff
-        attr_reader :raw
+        attr_reader :raw, :diff_type, :type, :title, :structure
 
         # Constructor: Accepts a diff in the traditional array format and stores it.
         # @param raw [Array] Diff in the traditional format
@@ -30,7 +30,21 @@ def initialize(raw)
           unless raw.is_a?(Array)
             raise ArgumentError, "OctocatalogDiff::API::V1::Diff#initialize expects Array argument (got #{raw.class})"
           end
+
           @raw = raw
+
+          unless ['+', '-', '~', '!'].include?(raw[0])
+            raise ArgumentError, 'Invalid first element array: diff type needs to be one of: +, -, ~, !'
+          end
+          @diff_type = raw[0]
+
+          unless raw[1].is_a?(String)
+            raise ArgumentError, "Invalid second element array: type-title-structure needs to be a string not #{raw[1].class}"
+          end
+          raw_1_split = raw[1].split(/\f/)
+          @type = raw_1_split[0]
+          @title = raw_1_split[1]
+          @structure = raw_1_split[2..-1]
         end
 
         # Public: Retrieve an indexed value from the array
@@ -39,12 +53,6 @@ def [](i)
           @raw[i]
         end
 
-        # Public: Get the change type
-        # @return [String] Change type symbol (~, !, +, -)
-        def diff_type
-          @raw[0]
-        end
-
         # Public: Is this an addition?
         # @return [Boolean] True if this is an addition
         def addition?
@@ -63,35 +71,17 @@ def change?
           diff_type == '~' || diff_type == '!'
         end
 
-        # Public: Get the resource type
-        # @return [String] Resource type
-        def type
-          @raw[1].split(/\f/)[0]
-        end
-
-        # Public: Get the resource title
-        # @return [String] Resource title
-        def title
-          @raw[1].split(/\f/)[1]
-        end
-
-        # Public: Get the structure of the resource as an array
-        # @return [Array] Structure of resource
-        def structure
-          @raw[1].split(/\f/)[2..-1]
-        end
-
         # Public: Get the "old" value, i.e. "from" catalog
         # @return [?] "old" value
         def old_value
-          return nil if addition?
+          return if addition?
           @raw[2]
         end
 
         # Public: Get the "new" value, i.e. "to" catalog
         # @return [?] "new" value
         def new_value
-          return nil if removal?
+          return if removal?
           return @raw[2] if addition?
           @raw[3]
         end
@@ -127,7 +117,7 @@ def new_line
         # Public: Get the "old" location, i.e. location in the "from" catalog
         # @return [Hash] <file:, line:> of resource
         def old_location
-          return nil if addition?
+          return if addition?
           return @raw[3] if removal?
           @raw[4]
         end
@@ -136,7 +126,7 @@ def old_location
         # @return [Hash] <file:, line:> of resource
         def new_location
           return @raw[3] if addition?
-          return nil if removal?
+          return if removal?
           @raw[5]
         end
 

spec/octocatalog-diff/tests/api/v1/catalog-diff_spec.rb

@@ -29,16 +29,16 @@
         expect(OctocatalogDiff::Util::Catalogs).to receive(:new).and_return(catalog_obj)
 
         diffs_obj = double
-        allow(diffs_obj).to receive(:diffs).and_return([['diff-1'], ['diff-2']])
+        allow(diffs_obj).to receive(:diffs).and_return([['+', ''], ['-', '']])
         expect(OctocatalogDiff::Cli::Diffs).to receive(:new).and_return(diffs_obj)
 
         logger, @logger_str = OctocatalogDiff::Spec.setup_logger
         @result = described_class.catalog_diff(logger: logger, node: 'foo')
       end
 
       it 'should return the expected data structure' do
-        expect(@result.diffs[0].raw).to eq(['diff-1'])
-        expect(@result.diffs[1].raw).to eq(['diff-2'])
+        expect(@result.diffs[0].raw).to eq(['+', ''])
+        expect(@result.diffs[1].raw).to eq(['-', ''])
         expect(@result.from.raw).to eq(@from_catalog)
         expect(@result.to.raw).to eq(@to_catalog)
       end

spec/octocatalog-diff/tests/api/v1/diff_spec.rb

@@ -357,7 +357,15 @@
     end
 
     it 'should raise ArgumentError if called with a non-array' do
-      expect { described_class.new('foo') }.to raise_error(ArgumentError)
+      expect { described_class.new('foo') }.to raise_error(ArgumentError, /initialize expects Array argument/)
+    end
+
+    it 'should raise ArgumentError if first element is not a valid diff type' do
+      expect { described_class.new(['chicken', '']) }.to raise_error(ArgumentError, /Invalid first element array/)
+    end
+
+    it 'should raise ArgumentError if second element is not a string' do
+      expect { described_class.new(['+']) }.to raise_error(ArgumentError, /Invalid second element array/)
     end
   end
 end