# Ruby wrapper for Instagram API

I've created my first gem as Instagram API wrapper https://github.com/agilie/instagram_api_gem. I would be very pleased if you give me some feedback on code organization, methods elegancy and so on.

The thing I concerned about the most is next. I have a module Resource

  module Resource
include Client

private

def show(id, options = {})
raise 'Resource ID is required' unless id
make_request resource_path(id), query: options
end

def index(id, resource_name, options = {})
raise 'Resource ID is required' unless id
make_request resource_path("#{id}/#{resource_name}"), query: options
end

def create(id, resource_name, options = {})
raise 'Resource ID is required' unless id
make_request(resource_path("#{id}/#{resource_name}"), { body: options }, :post)
end

def destroy(id, resource_name, resource_id = nil, options = {})
raise 'Resource ID is required' unless id
path = "#{id}/#{resource_name}"
path = "#{path}/#{resource_id}" if resource_id
make_request(resource_path(path), { query: options }, :delete)
end
end


And I use methods from it like this

  class Tag
include Search
include Resource

def initialize(tag_id = nil)
@tag_id = tag_id
end

def recent_media(options = {})
index @tag_id, 'media/recent', options
end
end


Does this implementation look good or are there more elegant solutions? I would be very pleased if you give me some feedback regarding the whole code organization and implementation, which could be found here https://github.com/agilie/instagram_api_gem

You are welcome to contribute if you prefer.

In the code you present here, I only see one mishaps :

raise 'Resource ID is required' unless id


is repeated multiple time and actually don't test for much. I'd consider making a private method that check id and also check if id is not empty (or even if it isn't strip.empty? to consider only-spaces strings).

For the create method, can the option parameters really can be an empty hash ? If not I'd consider at least removing the default value for this.

I haven't read your whole repo, so I'm not sure but does the Tag class really need to include both Search and Ressource ?

Code like

@user_id = user_id ? user_id : 'self'


should be replaced with @user_id = user_id || 'self'

One important point : your code must be hard to unit test as it is coupled with HTTParty, you should consider using a Strategy pattern here.

Finally, I'd make some simple class that have only one or two fields (such as Tag, Location, User...) immutable by duping and freezing the parameter and freezing the whole object : (note that it won't work if you expect a boolean, a number, a Symbol or nil as dup raise an exception in such case :()

def initialize(user_id)
@user_id = (user_id || 'self').dup
@user_id.freeze
freeze
end


I haven't done Ruby in some time so I'm a bit rusty but I hope my review helped you at least a bit :)

• Thanks a lot for your answer. I appreciate and will take all your recommendations into account. Jun 16, 2017 at 10:20
• @SergeyMell no problem :) Jun 16, 2017 at 16:00