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?

• max pleaner: Personally I find the call pattern a little opaque, you could easily replace the name call with something like deliver_message_to_slack and the behavior is more obvious

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