Change architecture of Ruby app to be more object oriented, readable

Question

I'm working on a Slack bot for service desk which sends direct message to user on a Slack when their ticket will be on user_action_needed status. I'm using AWS Lambda to handle Jira incoming webhooks. Everything works well but I think I've got an issue with whole architecture of the app - I quess the code is not so readable, name of the class probably doesn't match what they do.

First of all I've got the handler on AWS lambda:

  module JiraHandler
    extend self
    def handle(event:, _context:)
      Parsers::JiraParser.new(event).call
      { statusCode: 200 }
    end
  end

Parsers::JiraParser is responsible not only for parsing events but it calls another class which grabs userId from Slack, and then inside of GetUserId I've got another class which sends message to user. So at the end if you call Parsers::JiraParser class you will receive slack message instead of parsed data.

Details of what I wrote about each class below:

Parsers::JiraParser

module Parsers
  class JiraParser
    def initialize(event)
      payload = event['body']
      @event = JSON.parse(payload)
    end

    def call
      ::Slack::GetUserId.new(reporter_email, reporter_name, ticket_number).call
    end
    # reporter_email, reporter_name, ticket_number are methods to pull data by .dig from event hash

GetUserId

  class GetUserId
    SLACK_LOOKUP_BY_EMAIL = 'https://slack.com/api/users.lookupByEmail'

    def initialize(email, name, ticket_number)
      @email = email
      @name = name
      @ticket_number = ticket_number
    end

    def call
      user_id = set_slack_user.dig('user', 'id')
      ::Slack::SlackMessenger.new(user_id, name, ticket_number).call
    end

    def set_slack_user
      HTTParty.get(SLACK_LOOKUP_BY_EMAIL, options)
    end

SlackMessanger

module Slack
  class SlackMessenger
    SLACK_API_ENDPOINT = 'https://slack.com/api/chat.postMessage'

    def initialize(user_id, name, ticket_number)
      @user_id = user_id
      @name = name
      @ticket_number = ticket_number
    end

    def call
      HTTParty.post(SLACK_API_ENDPOINT, body: params, headers: headers)
    end

I don't think this is a good approach, should I create an extra class where all those classes will be called? or maybe I should use monads?

Answer 1

Your code is not very clear indeed, as you said the name of the classes don't reflect what they do.

GetUserId sends the message to the user, I can't infer that just by the name of the class.

You know the steps of your program, which is good, but you haven't separated them in your program, so, the first step would be to create each of these steps in an independent way and then create a class that coordinates what has to be called. I think the handle method can do this coordination, it doesn't need to be as clean as you did.

Also, try to create classes that have states and operations, this way it is more object-oriented IMO. In terms of DDD, you are creating Service classes, you can try to create Entity classes.

For example (just a draft):

slack_api = SlackAPI.new

user = ::Slack::User.new(reporter_email, reporter_name, ticket_number, slack_api.user_id)
message = ::Slack::Message.new(user)

slack_api.send_message!(message)

(I've introduced here a class that will handle the communication with the Slack API.)

User:

class Slack::User
  def initialize(email, name, ticket_number, id)
    @id = id
    @email = email
    @name = name
    @ticket_number = ticket_number
  end
end

Message:

class Slack::Message
  def initialize(user_id, name, ticket_number)
    @user_id = user_id
    @name = name
    @ticket_number = ticket_number
  end
end