Disposable Object
Have you ever pulled a plate out of the dishwasher and put it on the table, only to realize that there’s still old food caked on it? Objects can be like that sometimes: mutable state can mean unpleasant surprises when old data leaves “stains” that you didn’t expect. Learn how to get the benefits of OO while avoiding many of the state-based pitfalls, by treating your objects as disposable.
Video transcript & code
So we're running an ecommerce system, and we've got a controller action that handles when orders are placed.
As part of placing an order, a couple of email notifications are sent. There's a receipt for the order. And then there's a followup email which suggests some other products the shopper might like based on what they just purchased.
Both of these emails are sent using a service object of the EmailNotifier
class.
require "ostruct"
require "./email_notifier"
class OrderController
def initialize
@order = OpenStruct.new(number: 23)
end
def current_user
OpenStruct.new(name: "Bob Example")
end
def create
# ...do order processing...
@notifier = EmailNotifier.new
@notifier.send_receipt(current_user, @order)
@notifier.send_related_products(current_user, @order)
end
end
Our store is successful, and we steadily accumulate customers. As we do, a nagging bug develops. Every month we send out a promotional email newsletter to every customer who hasn't opted-out. We're not getting the number of email views and click-throughs we'd expect given the size of our database. When we dig into our SMTP logs, we discover that our promotion-sending job seems to mysteriously stop sending out emails partway through the list of customers.
When we take a look at the batch job that sends out the monthly letters, this is what we see.
This code first creates an EmailNotifier
. Then it iterates through each user account. For each one, it uses the notifier
to send them a monthly promotion email.
notifier = EmailNotifier.new
User.all.each do |user|
notifier.send_monthly_promo(user, featured_product_list)
end
Let's dive into the EmailNotifier
class to see if it holds the key to what's going on.
This class has methods for each type of email it knows how to send. We can see, for instance, that the send_receipt
method expects a user and an order object.
Then it assembles an email subject and body .
Finally, it delegates to an internal method called send_email
to do the actual sending. One thing we note here is that there is a keyword argument determining whether this email should be treated as promotional. In this case, it's false.
Let's scroll down to the method for sending monthly promo emails. The bulk of this method is similar in form to the one for sending receipts. But unlike for receipts, this one flags the email as promotional.
When we trace down into the send_email
method, we can see that first it puts together to
and from
email addresses. Then, it uses an Optout
model to look up the destination email address and check if that address has been opted-out of promotions.
Next there's a conditional. If this is a promotional email and the email address has been opted-out, no send will be performed. Otherwise, it will go ahead.
Looking again at the line that checks if the email has been opted-out, we finally spot the problem. This code uses an instance variable and or-equals assignment to idempotently cache the opt-out value the first time this line of code is run.
Presumably, someone saw that this was a line that could trigger a second database query, and decided to optimize it so it wouldn't re-query every time through. Remember how EmailNotifier
is used in the controller: it is used to send two different emails in a row, both to the same email address. In this case, the opt-out value caching works fine, because the customer is the same each time.
But the code for sending monthly newsletters works differently. Here, the same notifier object is used to send emails to many different email addresses.
For this scenario, the opt-out caching is doubly broken. First, because even it it worked as the writer presumably intended, it would apply the first user's email preferences to all customers. But since it's using an ||= operator with boolean values, it has an even more subtly incorrect behavior. It will work properly as long as it processes users who have not opted-out. Each false
value will be treated as if the variable is unset, and it will be re-set to false
over and over again.
But as soon as it hits a user who has opted-out, that true
value will lodge in the @opted_out
instance variable, and now that it is truthy no other value will budge it out. As a result, every customer processed after the first opted-out customer will also be treated as if they had opted out.
require "./optout"
require "./mailotron"
class EmailNotifier
def send_receipt(user, order)
subject = "Here's your receipt"
body = <<~EOF
Hi #{user.name},
Here's your receipt for order ##{order.number}:
...blah blah blah...
EOF
send_email(user, subject, body, promotional: false)
end
def send_related_products(user, order)
subject = "We have some suggestions for you!"
body = <<~EOF
Hey there #{user.name},
Based on your recent purchases, we have some
suggestions for you!
...yadda yadda yadda...
EOF
send_email(user, subject, body, promotional: true)
end
def send_monthly_promo(user, promoted_products)
subject = "Check out what's hot at AcmeCo!"
body = <<~EOF
What's up #{user.name}!
We've got some exciting products this month that you
won't want to miss!
...etc etc etc...
EOF
send_email(user, subject, body, promotional: true)
end
def send_email(user, subject, body, promotional:)
to = user.email
from = "AcmeCo <noreply@example.com"
@opted_out ||= Optout.find(email: to).opted_out?
unless promotional && @opted_out
Mailotron.send(to: to,
from: from,
subject: subject,
body: body)
end
end
end
There are plenty of code smells we could ding this code for: a query to a separate table buried deep in logic. Some possibly premature optimization. A naive use of the ||= conditional assignment operator.
But there's a deeper language design factor that allowed this bug to happen in the first place. It's a feature that is increasingly seen by many in the software industry as a fundamental flaw in object-oriented languages. It's the fact that objects have mutable state. When we have objects that can change themselves as a side effect of performing their roles, there's an infinite possibility for objects to behave in unpredictable ways. With mutable state, we can't rely on the same method call with the same arguments having the same outcome every single time. There's always the chance that, like with this example here, some change in state will unintentionally "bleed over" from an earlier operation to a later one.
There are some people who feel that, because of this flaw, we should either abandon stateful languages like Ruby for languages based on immutable data structures such as Haskell or Clojure. There are others who are seeking to bring immutable data structures and coding styles into languages such as Ruby.
Both of these are valid directions that I think are well worth exploring. But sometimes project constraints don't allow for a radical restructuring based on immutability principles, let alone a change in language. And the truth is, I believe that mutable vs. immutable isn't an either/or, black-or-white choice.
In my experience, we can often get 80% of the benefits of immutability not by removing all changeable state, but by minimizing the lifetime of stateful objects. Another way of saying this is to embrace disposable objects.
To treat EmailNotifier
as a disposable object back in the batch mailing code, we inline the notifier
variable. Now a new EmailNotifier
is created for each iteration. The buggy state-bleeding code is still there, but it doesn't matter anymore, because the objects are thrown away before it can cause any trouble.
User.all.each do |user|
EmailNotifier.new.send_monthly_promo(user, featured_product_list)
end
In order to avoid any future problems, we decide to enforce the use of EmailNotifier
as a disposable object. To do that, we add a guard clause to the send_email
method that checks a flag variable to see if this object has already been used to send an email. At the bottom of the method, we ensure
that the flag is set to true.
require "./email_notifier"
class EmailNotifier
# ...
def send_email(user, subject, body, promotional:)
fail "EmailNotifier cannot be reused!" if @is_used
to = user.email
from = "AcmeCo <noreply@example.com"
@opted_out ||= Optout.find(email: to).opted_out?
unless promotional && @opted_out
Mailotron.send(to: to,
from: from,
subject: subject,
body: body)
end
ensure
@is_used = true
end
end
This change causes other uses of EmailNotifier
to start blowing up, including the one in the OrderController
.
require "./controller"
require "./email_notifier2"
controller = OrderController.new
controller.create # ~> RuntimeError: EmailNotifier cannot be reused!
# ~> RuntimeError
# ~> EmailNotifier cannot be reused!
To fix this we inline the EmailNotifier
construction in the controller method.
require "ostruct"
require "./email_notifier"
class OrderController
def initialize
@order = OpenStruct.new(number: 23)
end
def current_user
OpenStruct.new(name: "Bob Example")
end
def create
# ...do order processing...
EmailNotifier.new.send_receipt(current_user, @order)
EmailNotifier.new.send_related_products(current_user, @order)
end
end
This code now feels a bit clunky. But instead of interpreting that as a sign that we're headed in the wrong direction, we can lean in and let it guide us to a better and more granular object decomposition.
The idea of disposable objects goes hand-in-hand with using smaller, single-purpose objects. As we find more time to refactor, we can move from an EmailNotifier
service object to a collection of smaller process objects, such as we saw in Episode #331, each one representing the act of sending a single email notification. In line with the notification-oriented naming convention we explored in that episode, rather than commanding these objects to send, we notify them that they are free to proceed with their purpose.
require "ostruct"
require "./email_notifier"
class OrderController
def initialize
@order = OpenStruct.new(number: 23)
end
def current_user
OpenStruct.new(name: "Bob Example")
end
def create
# ...do order processing...
ReceiptNotification.new(current_user, order: @order).proceed
RelatedProductsNotification.new(current_user, order: @order).proceed
end
end
Disposable objects are like disposable paper plates: we don't have to worry about accumulating a sticky, hard-to-scrub mess of state in them, because we're never going to reuse them. We use them once and then we throw them away. The next time you're fighting bugs caused by accumulated state, see if there's a way to reduce the lifetime of the objects involved. You may find it's cheaper to simply discard old, smelly state than it is to prevent it from building up in the first place. Happy hacking!
Responses