# Logging leads when interactions happen for a contact

I have been trying to get as close to SRP in my code as possible. I have a situation where I need to log leads when interactions happen for a contact. A lead may be created or updated. There are three different states that I would like to set leads to.

• Active - new interested interactions causes active lead to be created
• Converted - a existing lead is updated to converted if the product was purchased - this is managed with conversion flag
• Stale - a existing lead is updated to stale if the contact is not interested

class LeadGeneration

def initialize contact_id, options={}

options.reverse_merge!(conversion: false, interested:true)
@conversion = options[:conversion]
@interested = options[:interested]
end

def call
if conversion?
else
end
end

private

end

def conversion?
@conversion
end

def interested?
@interested
end

end

if interested?
else
end
end

end

end
end


Right now I feel like I should be splitting up converting a lead and managing stale/active states. I'm not really sure what to do and tests are a little confusing right now.

-

I'm not sure how you intend to use this class, but the call method is very fishy. This method seems to do one of three things: convert_lead, activate_lead or stale_lead. The choice of which of them should actually be executed is made in the options argument during object construction.

This is not very easy to understand from reading the code, and I can't really see the reasoning behind it. Is it an implementation of a Command pattern? that is not very ruby-like.

Since all this class actually does is quite basic delegation (@lead.converted!, @lead.stale! or @lead.active!) I would consider foregoing this class altogether, and simply doing the action. It is unnecessarily long and convoluted, and hides more than explains what it does.

From your comment below I understand that LeadGeneration should be a Service Object. It is true that service objects may have a single method called call, as is suggested in this blog post:

Service objects are responsible for retrieving and/or manipulating data — essentially any work that needs to be done that you might have put in a controller or model object before. They typically only have one method, which I like to define as “#call” (they're usually named something like PictureCreationService, so naming the method #create would seem redundant).

But note, though, the reason a single method call is allowable - it is apparent from the class name exactly what the method should do.
If your service object was called LeadConversionService, then a call method would be good enough.

Being a service object, it is even more important than usual in ruby for the class to be small and transparent.

If you want to go to the extreme SRP suggested in the quote above, you'd have three classes, each with #call and #allowed? methods:

class LeadConversionService
end

def call
end

def allowed?
end
end


OR, you could be a little less extreme, and decide the LeadGeneration (or maybe LeadLifeCycleService?) is a good enough scope, and let it have three method:

class LeadLifeCycleService
end

def convert!
end

def activate!
end

def stale!
end
end


Writing this solution makes it obvious again, that this service doesn't do much, since it only delegates (so it doesn't even save code in your model...). All it does is validates pre-conditions to your actions, which may be better handled by a state-machine, as @Flambino suggested.

-
It is a service object. I want to ensure that the logic regarding how leads are captured is stored within this service object. Otherwise I would need to pull it out into other objects. From my reading using 'call' is acceptable practice for calling service objects. I had all this logic in my models, but from my reading - it is bad practice to bloat your models with logic not related to basic CRUD. Abstractive it into it's own service object makes is much more testable. Sorry I should have said it is a service object in the question. –  Ryan-Neal Mes Apr 4 at 9:22
@Ryan-NealMes - updated my answer for service objects... –  Uri Agassi Apr 4 at 9:49
Does exposing #allowed? pass the responsibility of knowing to check if a lead can be converted outside the responsibility of the service class? would I not do @lead.converted! if allowed? [Thanks for clearing up so much for me!] –  Ryan-Neal Mes Apr 4 at 10:17
@Ryan-NealMes - The #allowed? method might be part of a policy, and not part of the service. The @lead.converted! if allowed? is not advisable, since it hides errors (as @Flambrino said). A better way would be to raise an error unless allowed?, or, at least expose a boolean as to whether the operation actually occurred. –  Uri Agassi Apr 4 at 10:24

Uri already had a good answer suggesting different approaches. I'd roll this into the Lead model too, and treat it as a straight-up state machine.

As far as I can tell, the states, in order, are:

Stale → Active (default) → Interested → Converted


Point being that you can't transition more than one step at a time (can't convert before being interested, can't be interested unless already active, etc.).

There are a number of state machine gems you can use to your advantage. Many (most?) include a simple DSL allowing you do perform the state transitions, trigger other code when a certain transition happens, etc.

Right now, you're passing either interested or conversion (or both?) to the initializer, which doesn't make sense without knowing the current state. The logic of your class hides all that, and doesn't provide much feedback (e.g. exceptions) if you attempt to do things "out of step" with the current state. For instance, if I pass interested: false, conversion: true what does that mean? Seems illogical. What if I pass conversion: true on a stale lead? These things make it confusing.

-
Interested and conversion are based off interactions the contact has with out application (you do not need to know the state of the lead). These interactions cause changes to the state in the lead. Eg if the interaction is not interested the lead if set to stale if it exists, if it doesn't exist no lead is captured. I pulled this out to a service object because it violates SRP if I put it into the lead or interaction models. If I put this stuff in my models I would have to use callbacks as well which is bad practice. –  Ryan-Neal Mes Apr 4 at 9:28