HTTP client for company services

For quite a while now I've been writing a bit of Ruby code, but so far I still don't think it was good idiomatic Ruby. But there's been a new requirement and I tried my best writing it better, more cleanly compared to my previous code. One of the things I did is read another code base that I though was well written (calabash-ios).

I would like to hear your opinions and suggestions for improvements.

```
module Company
module MobileServices
class Client
include DigestHelper

attr_reader :client_name, :client_digest_key

# headers that will be part of every request type
REQUEST_INIT_HEADER = {
'User-Agent' => 'Calabash/MobileServicesClient',
'Accept' => 'application/json'
}

# when creating a new instance, make sure the client name and digest key
# are correct as a digest key is linked to a specific client
def initialize(client_name = 'client_x', client_digest_key = 'some_random_key')
@client_name = client_name
@client_digest_key = client_digest_key
end

# execute request base on path; optionally add the following parameters
# - user_id: a registered email address for the target environment (host)
# - password: the password linked to the email address
# - host: default is some.host.nl, change as needed
# - http_method: GET (default), PUT, POST or DELETE are supported
# - body: the body of the request, only JSON content is supported
#
# PLEASE NOTE: we can't get URL's through proxy working, digest
# calculation seems to fail
def execute_request(path, options={})
default_options = {:host => 'some.host.nl',
:user_id => 'account@example.com',
:password => 'pa55w0rd',
:http_method => 'GET',
:body => ''}

options = default_options.merge(options)

digest = calc_digest(path

In (subjective) order or relevance:

You shouldn't really raise a RuntimeError (default):

raise "invalid or unsupported HTTP method: #{http_method}"

Instead, define your own exception class and raise it:

HttpMethodError = Class.new(StandardError)
...
raise HttpMethodError, "invalid method #{method}"

RuntimeError is pretty generic, and if something else throws it, your rescue statements could not tell the difference, making it hard to handle such exception.

Your methods are somewhat long thus unreadable. Find atomic tasks and extract them, i.e.

def request_for_method http_method
    case http_method
    when 'GET'
      Net::HTTP::Get.new(uri.request_uri, REQUEST_INIT_HEADER)
    when 'PUT'
      Net::HTTP::Put.new(uri.request_uri, REQUEST_INIT_HEADER)
    when 'POST'
      http_request = Net::HTTP::Post.new(uri.request_uri, REQUEST_INIT_HEADER)
    when 'DELETE'
      Net::HTTP::Delete.new(uri.request_uri, REQUEST_INIT_HEADER)
    else 
      raise "invalid or unsupported HTTP method: #{http_method}"
    end
  end

You could also split execute_request in at least two parts (logic in first, putses in second.

Newer version's of Ruby (2.0 up I think) would allow you to turn this:

def execute_request(path, options={})
    default_options = {:host => 'some.host.nl',
                       :user_id => 'account@email.com',
                       :password => 'pa55w0rd',
                       :http_method => 'GET',
                       :body => ''}

into this:

def execute_requests(path, host: 'some.host.nl', user_id: 'account@email.com' ...)

There's no need to check for nil explicitly:

unless http_request.nil?
# equals
if http_request

Likewise:

unless body == ''
# equals
unless body.empty?

Some people would argue that second option is more readable.

This is more like a hint, or an opinion than an issue: if you have such deeply nested code you can do things like:

class Company::MobileServices::Client
  include DigestHelper
  # ...
end

While I'd say it makes your code more readable (provided you only have single class in a file), in Ruby we most often only use two spaces to indent, so it's not really a problem.

Even if you don't extract this case in it's method (or if you do), don't assign to variable in each when. Ruby way to do this is to leverage the fact that case is an expression returning a value (like everything else is):

http_request =
      case http_method
      when 'GET'
        Net::HTTP::Get.new(uri.request_uri, REQUEST_INIT_HEADER)
      when 'PUT'
        Net::HTTP::Put.new(uri.request_uri, REQUEST_INIT_HEADER)
      when 'POST'
      # ...
      end

StackExchange Code Review Q#107234, answer score: 4