First go at best practice w/service objects after workshops. How does this class look?

Ruby on Rails
1

Hi all, my first post here - I’ve been using rails for over a year (after 6 years in PHP) but it was mostly a PHP in Ruby approach and after some feedback I decided to get involved here and get my head around the way things should be done for moving forward. I’ve done both the intro and intermediate workshops and learned a lot; now I’m on my first app trying to integrate everything I’ve learned so far.

I have a question on best practice with classes, specifically service objects. I am using omniauth for users to signin via reddit (I will be accessing the reddit API). I have stripped it out from the model and built a class that either fetches or create the user object. So this is the class:

class UserFromOmniauth

  def initialize(auth)
    @auth = auth
  end

  def create_or_fetch
    user = fetch_existing_user || create_new_user
    update_token(user)
    user
  end

  private

  def fetch_existing_user
    User.where(@auth.slice(:provider, :uid)).first
  end

  def create_new_user
    User.create(
      :provider => @auth.provider, 
      :uid => @auth.uid, 
      :name => @auth.info.name,
      :email => "reddit#{rand(0.500000).ceil}@test.com", 
      :password => Devise.friendly_token[0,20]
    )
  end

  def update_token(user)
    user.update_attribute(:token, @auth.credentials.token)
  end
end

And I’m calling it with (in omniauth_callback_controller.rb):

@user = UserFromOmniauth.new(request.env["omniauth.auth"]).create_or_fetch

Firstly any general feedback on the approach would be very helpful. But my main question is about using UserFromOmniauth.new(args).create_or_fetch.

As it needs to return the user object, doing things in the initialize function doesn’t work as it won’t return anything. I did have self.create_or_fetch at one stage, and called it with UserFromOmniauth.create_or_fetch(args), so there was no object.new.method and just object.method, but the service object itself had to be full of self.x’s, all methods had to be passed arguments with (args) rather than an instance variable, and so the class itself looked a lot less clean.

So, is creating an object with ‘new’ and then calling a method on it in the same line bad practice? Or does it look OK?

Many thanks for any and all input.

2

I use the foo = FooBuilder.new.build_foo(inputs) technique all the time; I think it’s quite readable.

No need to keep the FooBuilder around for longer than one line, unless you need to examine its internal structures for some reason.

Building a User from an Omniauth response is a perfect example. It has to be done only once per User object, so why clutter up the User object with dozens of lines of code that are irrelevant for most of the user’s life?

3

Thanks for the reply Matt and I’m pleased to hear that you think it’s a valid approach, especially as I now have about 15 service objects doing a similar thing when something needs returning :).

Thanks again!

1 Like