respond to codereview

lib/target_process.rb

@@ -3,7 +3,7 @@
 require 'target_process/api_error'
 require 'target_process/api_client'
 require 'target_process/base'
-Dir["./lib/target_process/entities/*.rb"].each {|file| require file }
+Dir["./lib/target_process/entities/*.rb"].each { |file| require file }
 
 module TargetProcess
   class ConfigurationError < StandardError; end

lib/target_process/api_client.rb

@@ -7,38 +7,33 @@ class APIClient
 
     def get(path, options = {})
       options.merge!(format: 'json')
-      options = { body: options }
-      response = perform(:get, path, options)
+      response = request(:get, path, query: options)
       normalize_response(response.parsed_response)
     end
 
     def post(path, attr_hash)
       content = prepare_data(attr_hash).to_json
       options = { body: content,
                   headers: { 'Content-Type' => 'application/json' } }
-      response = perform(:post, path, options)
+      response = request(:post, path, options)
       normalize_response(response.parsed_response)
     end
 
     def delete(path)
-      perform(:delete, path).response
+      request(:delete, path).response
     end
 
     private
 
-    def perform(type, path, options = {})
-      auth = { username: TargetProcess.configuration.username,
-               password: TargetProcess.configuration.password }
-      options.merge!(basic_auth: auth)
-      check_for_api_errors HTTParty.send(type, generate_url(path), options)
+    def request(type, path, options = {})
+      options.merge!(basic_auth)
+      response = HTTParty.send(type, generate_url(path), options)
+      check_for_api_errors(response)
+      response
     end
 
     def check_for_api_errors(response)
-      if response['Error']
-        raise APIError.parse(response)
-      else
-        response
-      end
+      raise APIError.parse(response) unless response.success?
     end
 
     def generate_url(path)
@@ -73,5 +68,10 @@ def prepare_data(hash)
       hash = Hash[hash.map { |k, v| [k.to_s.camelize.to_sym, v] }]
       hash.each { |k, v| hash[k] = json_date(v) if v.is_a?(::Time) }
     end
+
+    def basic_auth
+      { basic_auth: { username: TargetProcess.configuration.username,
+                      password: TargetProcess.configuration.password } }
+    end
   end
 end

lib/target_process/api_error.rb

@@ -11,7 +11,7 @@ class Unauthorized < APIError; end
     def self.parse(response)
       error = response['Error']
       status = error['Status'] || response['Status'] || 'Undefined'
-      message = raw_message(response.parsed_response)
+      message = raw_message(response)
       type = "#{self}::#{status}".safe_constantize
       constants.include?(status.to_sym) ? type.new(message) : new(message)
     end

lib/target_process/base.rb

@@ -105,11 +105,11 @@ def self.has_many(name, klass = nil)
       klass ||= name.to_s.singularize.camelize
       define_method(name) do
         path = entity_path + name.to_s.camelize
-        collection = TargetProcess.client.get(path)[:items].collect do |hash|
-                       entity = "TargetProcess::#{klass}".constantize.new
-                       entity.attributes.merge!(hash)
-                       entity
-                     end
+        TargetProcess.client.get(path)[:items].collect do |hash|
+          entity = "TargetProcess::#{klass}".constantize.new
+          entity.attributes.merge!(hash)
+          entity
+        end
       end
     end