Move location validation logic from class to instance methods

This makes validating locations more intuitive.
It also separates building a location from validating it.

lib/coffee_place/cli.rb

@@ -51,13 +51,12 @@ def parse_cli_options!(args)
 
     def validate_user_location!(cli_opts)
       lat, lon = cli_opts.remaining
-      location_result = Location.validate(lat: lat, lon: lon, name: 'Current user')
+      location = Location.new(lat: lat, lon: lon, name: 'Current user')
 
-      if location_result.success?
-        location_result.value
+      if location.valid?
+        location
       else
-        puts "Failure: #{location_result.error}"
-        show_help_and_exit!(exit_status: 1)
+        print_errors_and_exit!(location.errors)
       end
     end
 

lib/coffee_place/importer.rb

@@ -29,13 +29,14 @@ def import_from(source_name)
 
     def import_line(line, line_index)
       place_name, lat, lon = line
+      location = Location.new(name: place_name, lat: lat, lon: lon)
 
-      result = Location.validate(name: place_name, lat: lat, lon: lon)
-
-      if result.success?
-        @locations << result.value
+      if location.valid?
+        @locations << location
       else
-        add_import_error "CSV error on line #{line_index}: #{result.error}"
+        location.errors.each do |error|
+          add_import_error "CSV error on line #{line_index}: #{error}"
+        end
       end
     end
 

lib/coffee_place/location.rb

@@ -10,7 +10,7 @@ class Location
     include Geo
     include EarthDistance
 
-    attr_accessor :lat, :lon, :name
+    attr_accessor :lat, :lon, :name, :errors
 
     def initialize(lat:, lon:, name:)
       @lat = lat
@@ -23,10 +23,14 @@ def distance_to(other_location)
     end
 
     def valid_latitude?
+      return false unless @lat
+
       within_latitude_bounds?(@lat)
     end
 
     def valid_longitude?
+      return false unless @lon
+
       within_longitude_bounds?(@lon)
     end
 
@@ -42,34 +46,57 @@ def same_place?(other)
       (@lat == other.lat) && (@lon == other.lon)
     end
 
-    class << self
-      def validate(lat:, lon:, name:)
-        result_lat = parse_coordinate(lat, 'latitude')
-        return result_lat unless result_lat.success?
+    def valid?
+      check_validation_errors
+      @errors.empty?
+    end
 
-        result_lon = parse_coordinate(lon, 'longitude')
-        return result_lon unless result_lon.success?
+    private
 
-        location = new(lat: result_lat.value, lon: result_lon.value, name: name.to_s)
+    def check_validation_errors
+      @errors = []
 
-        return Result.failure("Invalid latitude: #{lat}") unless location.valid_latitude?
-        return Result.failure("Invalid longitude: #{lon}") unless location.valid_longitude?
+      normalize_lat
+      normalize_lon
 
-        Result.success(location)
-      rescue ArgumentError => e
-        Result.failure(e.message)
-      end
+      # Guard clause returning early in case latitude / longitude are invalid
+      return if @errors.any?
 
-      private
+      @errors << "Invalid latitude: #{lat}" unless valid_latitude?
+      @errors << "Invalid longitude: #{lon}" unless valid_longitude?
 
-      def parse_coordinate(coordinate, label)
-        return Result.failure("Missing #{label}") unless coordinate
+      @errors
+    end
+
+    def normalize_lat
+      result = parse_coordinate(lat, 'latitude')
 
-        value = Float(coordinate)
-        Result.success(value)
-      rescue ArgumentError
-        Result.failure("Invalid #{label}: #{coordinate.inspect}")
+      if result.success?
+        @lat = result.value
+      else
+        @lat = nil
+        @errors << result.error
       end
     end
+
+    def normalize_lon
+      result = parse_coordinate(lon, 'longitude')
+
+      if result.success?
+        @lon = result.value
+      else
+        @lon = nil
+        @errors << result.error
+      end
+    end
+
+    def parse_coordinate(coordinate, label)
+      return Result.failure("Missing #{label}") unless coordinate
+
+      value = Float(coordinate)
+      Result.success(value)
+    rescue ArgumentError
+      Result.failure("Invalid #{label}: #{coordinate.inspect}")
+    end
   end
 end

spec/coffee_place/importer_spec.rb

@@ -56,6 +56,7 @@
         expect(result).not_to be_success
         expect(result.error).to contain_exactly(
           'CSV error on line 2: Invalid latitude: " South America"',
+          'CSV error on line 2: Invalid longitude: " Western Hemisphere"',
           'CSV error on line 4: Missing longitude'
         )
       end

spec/coffee_place/location_spec.rb

@@ -120,70 +120,51 @@
     end
   end
 
-  describe '.validate' do
-    subject { described_class }
+  describe '.validation' do
+    subject(:location) { described_class.new(lat: 22.3, lon: 42.9, name: 'Funkytown') }
 
-    let(:valid_opts) do
-      { lat: 22.3, lon: 42.9, name: 'Funkytown' }
-    end
-
-    it 'returns success with location when options valid' do
-      result = subject.validate(**valid_opts)
-
-      expect(result).to be_success
-      expect(result.value).to have_attributes(
-        lat: 22.3,
-        lon: 42.9,
-        name: 'Funkytown'
-      )
-    end
+    it { is_expected.to be_valid }
 
-    it 'returns failure when latitude is invalid' do
-      invalid_opts = valid_opts.merge(lat: -91.2)
-      result = subject.validate(**invalid_opts)
+    it 'returns error when latitude is invalid' do
+      location.lat = -91.2
 
-      expect(result).to be_failure
-      expect(result.error).to eq('Invalid latitude: -91.2')
+      expect(location).not_to be_valid
+      expect(location.errors).to include('Invalid latitude: -91.2')
     end
 
     it 'returns failure when longitude is invalid' do
-      invalid_opts = valid_opts.merge(lon: 182.3)
-      result = subject.validate(**invalid_opts)
+      location.lon = 182.3
 
-      expect(result).to be_failure
-      expect(result.error).to eq('Invalid longitude: 182.3')
+      expect(location).not_to be_valid
+      expect(location.errors).to include('Invalid longitude: 182.3')
     end
 
     it 'returns failure when latitude cannot be parsed' do
-      invalid_opts = valid_opts.merge(lat: '7 cats')
-      result = subject.validate(**invalid_opts)
+      location.lat = '7 cats'
 
-      expect(result).to be_failure
-      expect(result.error).to eq('Invalid latitude: "7 cats"')
+      expect(location).not_to be_valid
+      expect(location.errors).to include('Invalid latitude: "7 cats"')
     end
 
     it 'returns failure when longitude cannot be parsed' do
-      invalid_opts = valid_opts.merge(lon: '10 passes North')
-      result = subject.validate(**invalid_opts)
+      location.lon = '10 passes North'
 
-      expect(result).to be_failure
-      expect(result.error).to eq('Invalid longitude: "10 passes North"')
+      expect(location).not_to be_valid
+      expect(location.errors).to include('Invalid longitude: "10 passes North"')
     end
 
     it 'returns failure when latitude is missing' do
-      invalid_opts = valid_opts.merge(lat: nil)
-      result = subject.validate(**invalid_opts)
+      location.lat = nil
 
-      expect(result).to be_failure
-      expect(result.error).to eq('Missing latitude')
+      expect(location).not_to be_valid
+      expect(location.errors).to include('Missing latitude')
     end
 
     it 'returns failure when longitude is missing' do
-      invalid_opts = valid_opts.merge(lon: nil)
-      result = subject.validate(**invalid_opts)
+      location.lon = nil
 
-      expect(result).to be_failure
-      expect(result.error).to eq('Missing longitude')
+      expect(location).not_to be_valid
+      expect(location.errors).to include('Missing longitude')
     end
   end
 end