From aaff583f8a722db49ff44a8225680027d981a96f Mon Sep 17 00:00:00 2001
From: Nathan Kleyn <nathan@nathankleyn.com>
Date: Wed, 5 Jun 2013 21:56:42 +0100
Subject: [PATCH] Cleaning up accessor code, more unit tests for NxtBrick.

---
 lib/lego-nxt.rb            |  2 +-
 lib/nxt/nxt_brick.rb       | 24 +++++-----
 lib/nxt/utils/accessors.rb | 11 ++---
 spec/nxt/nxt_brick_spec.rb | 92 +++++++++++++++++++++++---------------
 4 files changed, 72 insertions(+), 57 deletions(-)

diff --git a/lib/lego-nxt.rb b/lib/lego-nxt.rb
index 60b5bce..ddfc259 100644
--- a/lib/lego-nxt.rb
+++ b/lib/lego-nxt.rb
@@ -6,8 +6,8 @@
 
 require 'nxt/exceptions'
 
-require 'nxt/utils/accessors'
 require 'nxt/utils/assertions'
+require 'nxt/utils/accessors'
 
 require 'nxt/interfaces/base'
 require 'nxt/interfaces/usb'
diff --git a/lib/nxt/nxt_brick.rb b/lib/nxt/nxt_brick.rb
index 8ab9079..6eb1097 100644
--- a/lib/nxt/nxt_brick.rb
+++ b/lib/nxt/nxt_brick.rb
@@ -51,30 +51,28 @@ def initialize(interface_type, *interface_args)
       raise InvalidInterfaceError.new("There is no interface of type #{interface_type}.")
     end
 
-    self.interface = NXT::Interface.const_get(interface_type).new(*interface_args)
+    @interface = NXT::Interface.const_get(interface_type).new(*interface_args)
 
     if block_given?
       begin
-        self.connect
+        connect
         yield(self)
-      rescue Exception => e
-        puts e
       ensure
-        self.disconnect
+        disconnect
       end
     end
   end
 
   # Connect using the given interface to the NXT brick.
   def connect
-    self.interface.connect
+    @interface.connect
   end
 
   # Close the connection to the NXT brick, and dispose of any resources that
   # this instance of NXTBrick is using. Any commands run against this runner
   # after calling disconnect will fail.
   def disconnect
-    self.interface.disconnect
+    @interface.disconnect
   end
 
   # Add a new connector instance, binding a specific identifier to the given
@@ -95,8 +93,8 @@ def add(port, identifier, klass)
     assert_responds_to('identifier', identifier, :to_sym)
     assert_type('klass', klass, Class)
 
-    if self.respond_to?(identifier)
-      if self.instance_variable_get(:"@#{port}").nil?
+    if respond_to?(identifier)
+      if instance_variable_get(:"@#{port}").nil?
         raise InvalidIdentifierError.new("Cannot use identifier #{identifier}, a method on #{self.class} is already using it.")
       else
         raise PortTakenError.new("Port #{port} is already set, call remove first")
@@ -107,7 +105,7 @@ def add(port, identifier, klass)
   end
 
   # Remove the assigned (if any) connector instance from the given
-  # identifier.
+  # identifier.interface
   #
   # @param Symbol identifier The identifier to search for and remove.
   def remove(identifier)
@@ -130,7 +128,7 @@ def remove(identifier)
       # people don't pass in the correct number of params, it says helpfully
       # '1 of 2' args passed (or something similar).
       define_method("add_#{const.to_s.underscore}_#{type_const.to_s.underscore}") do |port, identifier|
-        self.add(port, identifier, NXT::Connector.const_get(type_const).const_get(const))
+        add(port, identifier, NXT::Connector.const_get(type_const).const_get(const))
       end
     end
   end
@@ -142,7 +140,7 @@ def define_port_handler_method(port, identifier, klass)
 
     # Makes a new instance of the class and pushes it into our instance variable
     # for the given port.
-    self.instance_variable_set(port_variable, klass.new(port, self.interface))
+    instance_variable_set(port_variable, klass.new(port, interface))
 
     # Given that that succeeded, all that remains is to add the identifier
     # to our lookup Hash. We'll use this Hash later on within method_missing.
@@ -150,7 +148,7 @@ def define_port_handler_method(port, identifier, klass)
 
     # Define a method on the eigenclass of this instance.
     (class << self; self; end).send(:define_method, identifier.to_sym) do
-      self.instance_variable_get(port_variable)
+      instance_variable_get(port_variable)
     end
   end
 end
diff --git a/lib/nxt/utils/accessors.rb b/lib/nxt/utils/accessors.rb
index 4c8a067..0134b4c 100644
--- a/lib/nxt/utils/accessors.rb
+++ b/lib/nxt/utils/accessors.rb
@@ -1,15 +1,12 @@
 module NXT
   module Utils
     module Accessors
+      include NXT::Utils::Assertions
+
       def attr_setter(name, options)
         define_method("#{name}=") do |value|
-          if options.include?(:is)
-            raise TypeError.new('Expected value to be a number') unless duration.is_a?(options[:is])
-          end
-
-          if options.include?(:is_key_in) && !options[:is_key_in].include?(value)
-            raise TypeError.new("Expected value to be one of: :#{options[:is_key_in].keys.join(', :')}")
-          end
+          assert_type(name, value, options[:is]) if options.include?(:is)
+          assert_in(name, value, options[:is_key_in]) if options.include?(:is_key_in)
 
           instance_variable_set("@#{name}", value)
           self
diff --git a/spec/nxt/nxt_brick_spec.rb b/spec/nxt/nxt_brick_spec.rb
index 05ad58c..7ea2f98 100644
--- a/spec/nxt/nxt_brick_spec.rb
+++ b/spec/nxt/nxt_brick_spec.rb
@@ -77,56 +77,78 @@
     end
   end
 
+  describe '#connect' do
+    before do
+      @interface = Object.new
+      subject.instance_variable_set(:@interface, @interface)
+    end
+
+    it 'should call connect on the interface' do
+      @interface.should_receive(:connect)
+      subject.connect
+    end
+  end
+
+  describe '#disconnect' do
+    before do
+      @interface = Object.new
+      subject.instance_variable_set(:@interface, @interface)
+    end
+
+    it 'should call disconnect on the interface' do
+      @interface.should_receive(:disconnect)
+      subject.disconnect
+    end
+  end
+
   describe '#add' do
     before do
+      @port = :a
+      @identifier = :hello
+      @class_stub = Class.new
+      @class_stub.stub(:new)
+
       subject.stub(:define_port_handler_method)
     end
 
     it 'should raise an exception if an invalid port number or letter is given' do
       expect do
-        subject.add(:invalid_port, :symbol, Class)
+        subject.add(:invalid_port, @identifier, @class_stub)
       end.to raise_exception(TypeError, 'Expected port to be one of: :a, :b, :c, :one, :two, :three, :four')
     end
 
     it 'should raise an exception if an invalid type of identifier is given' do
       expect do
-        subject.add(:a, 123, Class)
+        subject.add(@port, 123, @class_stub)
       end.to raise_exception(TypeError, 'Expected identifier to respond to: to_sym')
     end
 
     it 'should raise an exception if an invalid type of klass is given' do
       expect do
-        subject.add(:a, :symbol, 'not a class')
+        subject.add(@port, @identifier, 'not a class')
       end.to raise_exception(TypeError, 'Expected klass to be of type Class')
     end
 
-    it 'should raise an exception if the port given is already set' do
-      port = :a
-
-      class_stub = Class.new
-      class_stub.stub(:new)
-      subject.stub(:hello)
-      subject.instance_variable_set(:"@#{port}", 'some value already there')
+    it 'should raise an exception if trying to use an identifier that is the name of a defined methodz' do
+      subject.stub(@identifier)
 
       expect do
-        subject.add(port, :hello, class_stub)
-      end.to raise_error(PortTakenError, "Port #{port} is already set, call remove first")
+        subject.add(@port, @identifier, @class_stub)
+      end.to raise_error(InvalidIdentifierError, "Cannot use identifier #{@identifier}, a method on NXTBrick is already using it.")
     end
 
-    it 'should raise an exception if trying to use an identifier that is the name of a defined methodz' do
-      port = :a
-      identifier = :hello
-
-      class_stub = Class.new
-      class_stub.stub(:new)
-      subject.stub(identifier)
+    it 'should raise an exception if the port given is already set' do
+      subject.stub(@identifier)
+      subject.instance_variable_set(:"@#{@port}", 'some value already there')
 
       expect do
-        subject.add(port, :hello, class_stub)
-      end.to raise_error(InvalidIdentifierError, "Cannot use identifier #{identifier}, a method on NXTBrick is already using it.")
+        subject.add(@port, @identifier, @class_stub)
+      end.to raise_error(PortTakenError, "Port #{@port} is already set, call remove first")
     end
 
     it 'should call #define_port_handler_method' do
+      subject.should_receive(:define_port_handler_method).with(@port, @identifier, @class_stub)
+      subject.add(@port, @identifier, @class_stub)
     end
   end
 
@@ -163,29 +185,27 @@
   end
 
   describe '#define_port_handler_method' do
+    before do
+      @port = :a
+      @identifier = :hello
+      @class_stub = Class.new
+      @class_stub.stub(:new)
+    end
+
     it 'should create a new instance of the passed klass and store it in the attribute for the given port' do
-      port = :a
-      class_stub = Class.new
       class_return_stub = stub()
-
-      class_stub.should_receive(:new) do
+      @class_stub.should_receive(:new) do
         class_return_stub
-      end.with(port, an_instance_of(NXT::Interface::Usb)).once()
+      end.with(@port, an_instance_of(NXT::Interface::Usb)).once()
 
-      subject.send(:define_port_handler_method, port, :hello, class_stub)
+      subject.send(:define_port_handler_method, @port, @identifier, @class_stub)
 
-      subject.send(port).should equal(class_return_stub)
+      subject.send(@port).should equal(class_return_stub)
     end
 
     it 'should set up the port identifiers correctly' do
-      port = :a
-      identifier = :hello_world
-      class_stub = Class.new
-      class_stub.stub(:new)
-
-      subject.send(:define_port_handler_method, port, identifier, class_stub)
-
-      subject.port_identifiers[identifier].should equal(port)
+      subject.send(:define_port_handler_method, @port, @identifier, @class_stub)
+      subject.port_identifiers[@identifier].should equal(@port)
     end
   end
 end