Refactor empty_subfield_fix for memory usage (#119)

lib/marc_cleanup/variable_fields.rb

@@ -510,28 +510,11 @@ def empty_indicator_fix(record)
 
   ### Remove empty subfields from DataFields
   def empty_subfield_fix(record)
-    fields_to_delete = []
-    curr_field = -1
     record.fields.each do |field|
-      curr_field += 1
       next unless field.class == MARC::DataField
-
-      curr_subfield = 0
-      subfields_to_delete = []
-      field.subfields.each do |subfield|
-        subfields_to_delete.unshift(curr_subfield) if subfield.value.empty? || subfield.value.nil?
-        curr_subfield += 1
-      end
-      subfields_to_delete.each do |i|
-        record.fields[curr_field].subfields.delete_at(i)
-      end
-      fields_to_delete.unshift(curr_field) if record.fields[curr_field].subfields.empty?
-    end
-    unless fields_to_delete.empty?
-      fields_to_delete.each do |i|
-        record.fields.delete_at(i)
-      end
+      field.subfields.delete_if { |subfield| subfield.value.nil? || subfield.value.empty? }
     end
+    record.fields.delete_if { |field| field.class == MARC::DataField && field.subfields.empty? }
     record
   end
 

spec/variable_fields/empty_subfield_spec.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'nokogiri'
+require 'marc'
+require 'marc_cleanup'
+require 'byebug'
+
+RSpec.describe 'empty_subfield_fix method' do
+  describe 'when there are empty subfields' do
+    it 'removes them' do
+      original_hash = { 'fields' => [{
+        '020' => { 'indicator1' => ' ',
+                   'indicator2' => ' ',
+                   'subfields' => [{ 'a' => '9780316458759',
+                                     'b' => nil,
+                                     'c' => '',
+                                     'd' => nil,
+                                     'e' => '',
+                                     'z' => '' }] }
+      }, {
+        '035' => { 'indicator1' => ' ',
+                   'indicator2' => ' ',
+                   'subfields' => [{ 'a' => 'ocn123',
+                                     'b' => nil,
+                                     'c' => '4567' }] }
+      }] }
+      original = MARC::Record.new_from_hash(original_hash)
+
+      expected_hash = { 'fields' => [{
+        '020' => { 'indicator1' => ' ',
+                   'indicator2' => ' ',
+                   'subfields' => [{ 'a' => '9780316458759' }] }
+      }, {
+        '035' => { 'indicator1' => ' ',
+                   'indicator2' => ' ',
+                   'subfields' => [{ 'a' => 'ocn123',
+                                     'c' => '4567' }] }
+      }] }
+      expected = MARC::Record.new_from_hash(expected_hash)
+
+      fixed = MarcCleanup.empty_subfield_fix(original)
+      expect(fixed).to eq(expected)
+    end
+  end
+end