From f4034d9d176a6402de135acd91dc34d7b39dbf4a Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Mon, 25 Sep 2017 12:16:04 +0200 Subject: ModelAttribute.classes: Move `#uniq` call higher Call `#uniq` right after mapping over the `klass` attribute. Recommendation from Robert (thanks!). This means there are less elements to deal with in the subsequent maps. Refs #4401 --- lib/model_attribute.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/model_attribute.rb b/lib/model_attribute.rb index 60580e306..9d821f551 100644 --- a/lib/model_attribute.rb +++ b/lib/model_attribute.rb @@ -12,9 +12,9 @@ class ModelAttribute def self.classes all .map(&:klass) + .uniq .map(&:to_s) .map(&:camelize) - .uniq end def self.group_by_class -- cgit v1.2.3 From 75c7bc19c318ace8136ee94c2a09952226c0b7de Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Mon, 25 Sep 2017 12:22:29 +0200 Subject: ModelAttribute#==: Check for matching class type Thanks to Robert for this suggestion, of course it makes a lot of sense to verify class equivalence when checking equality. Refs #4401 --- lib/model_attribute.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/model_attribute.rb b/lib/model_attribute.rb index 9d821f551..4d246853a 100644 --- a/lib/model_attribute.rb +++ b/lib/model_attribute.rb @@ -93,7 +93,8 @@ class ModelAttribute end def ==(other) - klass == other.klass && + self.class === other && + klass == other.klass && name == other.name && data_type == other.data_type end -- cgit v1.2.3 From bcae693304405cc9c273cf60952b1d123a820e31 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Mon, 25 Sep 2017 12:29:25 +0200 Subject: ModelAttribute spec: Move `#instance_variable_set` to `before` block Recommendation from Robert. The `@__all__` is an implementation detail. It's more fragile to use `#instance_variable_set` in each test. Instead, put that part in a `before` block so that if we decide to change it, we only have to do so in one place. Now each test can blithely assume that `ModelAttribute.all` starts out as an empty list. Refs #4401 --- spec/lib/model_attribute_spec.rb | 46 +++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/spec/lib/model_attribute_spec.rb b/spec/lib/model_attribute_spec.rb index 427e01490..cdba87a90 100644 --- a/spec/lib/model_attribute_spec.rb +++ b/spec/lib/model_attribute_spec.rb @@ -1,4 +1,8 @@ RSpec.describe ModelAttribute do + before(:each) do + ModelAttribute.instance_variable_set(:@__all__, []) + end + describe ".define" do it "adds a new instance of ModelAttribute to .all" do expect do @@ -16,11 +20,9 @@ RSpec.describe ModelAttribute do describe ".classes" do it "returns the list of classes of ModelAttributes in .all" do - ModelAttribute.instance_variable_set(:@__all__, [ - ModelAttribute.new(:route, :name, :string), - ModelAttribute.new(:journey_pattern, :name, :string), - ModelAttribute.new(:time_table, :start_date, :date) - ]) + ModelAttribute.define(:route, :name, :string) + ModelAttribute.define(:journey_pattern, :name, :string) + ModelAttribute.define(:time_table, :start_date, :date) expect(ModelAttribute.classes).to match_array([ 'Route', @@ -32,9 +34,7 @@ RSpec.describe ModelAttribute do describe ".from_code" do it "returns a ModelAttribute from a given code" do - ModelAttribute.instance_variable_set(:@__all__, [ - ModelAttribute.new(:journey_pattern, :name, :string) - ]) + ModelAttribute.define(:journey_pattern, :name, :string) expect(ModelAttribute.from_code('journey_pattern#name')).to eq( ModelAttribute.new(:journey_pattern, :name, :string) @@ -44,12 +44,10 @@ RSpec.describe ModelAttribute do describe ".group_by_class" do it "returns all ModelAttributes grouped by klass" do - ModelAttribute.instance_variable_set(:@__all__, [ - ModelAttribute.new(:route, :name, :string), - ModelAttribute.new(:route, :published_name, :string), - ModelAttribute.new(:journey_pattern, :name, :string), - ModelAttribute.new(:vehicle_journey, :number, :integer) - ]) + ModelAttribute.define(:route, :name, :string) + ModelAttribute.define(:route, :published_name, :string) + ModelAttribute.define(:journey_pattern, :name, :string) + ModelAttribute.define(:vehicle_journey, :number, :integer) expect(ModelAttribute.group_by_class).to eq({ route: [ @@ -68,12 +66,10 @@ RSpec.describe ModelAttribute do describe ".methods_by_class" do it "returns all ModelAttributes for a given class" do - ModelAttribute.instance_variable_set(:@__all__, [ - ModelAttribute.new(:route, :name, :string), - ModelAttribute.new(:route, :published_name, :string), - ModelAttribute.new(:route, :direction, :string), - ModelAttribute.new(:journey_pattern, :name, :string) - ]) + ModelAttribute.define(:route, :name, :string) + ModelAttribute.define(:route, :published_name, :string) + ModelAttribute.define(:route, :direction, :string) + ModelAttribute.define(:journey_pattern, :name, :string) expect(ModelAttribute.methods_by_class(:route)).to match_array([ ModelAttribute.new(:route, :name, :string), @@ -85,12 +81,10 @@ RSpec.describe ModelAttribute do describe ".methods_by_class_and_type" do it "returns ModelAttributes of a certain class and type" do - ModelAttribute.instance_variable_set(:@__all__, [ - ModelAttribute.new(:route, :name, :string), - ModelAttribute.new(:route, :checked_at, :date), - ModelAttribute.new(:journey_pattern, :name, :string), - ModelAttribute.new(:journey_pattern, :section_status, :integer) - ]) + ModelAttribute.define(:route, :name, :string) + ModelAttribute.define(:route, :checked_at, :date) + ModelAttribute.define(:journey_pattern, :name, :string) + ModelAttribute.define(:journey_pattern, :section_status, :integer) expect(ModelAttribute.methods_by_class_and_type(:route, :string)).to match_array([ ModelAttribute.new(:route, :name, :string) -- cgit v1.2.3