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?

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 — max pleaner
– max pleaner, Commented Feb 14, 2020 at 17:41

m26a · Accepted Answer · 2020-04-15 21:57:41Z

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

Stack Exchange Network

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

1 Answer 1

You must log in to answer this question.

Hot Network Questions

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

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions