1
\$\begingroup\$

I have this piece of Ruby code:

HTTPCLIENT = HTTPClient.new

def read_page(url)
  HTTPCLIENT.get(url).body
end

The HTTPClient object is being created and declared as a constant, and thus it can be accessible by methods.

I would do this by declaring it as a global variable ($) or as a class variable (@), but I'm not sure if it's actually wrong the way it is.

I don't declare objects as variables because, well, their memory contents can vary. I only use constants to declare simple types (strings, numbers) or classes and modules. But objects? I'm not sure about it.

My question is: is this a bad code or it's just me? If so, then why do you think is a bad code?

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

I'd propose a 3rd way: Make it a method:

def client
  @client ||= HTTPClient.new
end

def read_page(url)
  client.get(url).body
end

There are some advantages to this:

  • You get the same scoping as with a constant (which you wouldn't get with a $-global), as well as visibility modifiers.

  • It's flexible. You can return anything that fulfils the API contract. It's the Dependency Inversion Principle (the D in SOLID) of relying on an abstraction. It also allows you to stub/mock the method and return value in tests much more easily.

  • If you want it to be shared across instances, use object composition (e.g. PageReader.new(existing_client)); if you want different instances to have their own client, use the implementation above.

I'd agree that constants should for be kept "primitive" - or at any rate, they should be data-only. E.g. a Hash, while not "primitive" per se, used as constant lookup table is fine. But a HTTP client instance is a bit much.

A 4th option is to make it a singleton, which is more or less a "constant instance". It'd probably require you to wrap (or extend) the HTTPClient class, e.g.:

require 'singleton'

class Client
  include Singleton

  def initialize
    @client = HTTPClient.new
  end

  def get(url)
    @client.get(url)
  end
end

# ...

def read_page(url)
  Client.instance.get(url).body
end
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Also consider using module, that is already in fact a singleton pattern, not a hack/crutch like include Singleton. \$\endgroup\$
    – Nakilon
    Jul 18, 2016 at 22:40

Not the answer you're looking for? Browse other questions tagged or ask your own question.