27

I have a function with a sensitive operation:

function doSomeThingSensitive() {
    customers = getAllCustomers()
    for each customer in customers
        if customer is from Europe
            Give them a 1000 euro giveaway coupon
}

I want to dry run this function in order to calculate how many sensitive operations (aka the 1000-euro giveaways) will be performed. The dry run function will look something like:

function doSomeThingSensitiveDryRun() {
    counter = 0
    customers = getAllCustomers()
    for each customer in customers
        if customer is from Europe
            counter++
    print 'we will pay ' + counter * 1000
}

The problem with this approach is that the dry run function is not maintainable at all. Any slight change in the initial function will break the logic on the dry run function, which makes its maintenance very difficult. Is there a different way I can approach this?

An idea is to run the initial function as a unit test and stub the sensitive operations. A different idea can be to run the initial function with a Boolean parameter, dryRun, and if it is set to true, the sensitive operation will not happen. Is there a better and maybe simpler way I can tackle this?

5 Answers5

57

You're doing multiple different things in that function. Start off by splitting it into separate functions. Here's an example using a closure for the "dry run" function.

function doWithCustomers(action)
    customers = getAllCustomers()
    for each customer in customers
        if customer is from Europe
            action(customer)

function sendCoupon(customer) give coupon to customer

Then you can provide the appropriate action depending on whether you're testing or running in production:

function main()
    doWithCustomers(sendCoupon)

function dryRun() count=0 doWithCustomers(() -> count++) print 'we will pay' + counter * 1000

It should be fairly trivial to convert the closure to an object if your language has the latter but lacks the former.

For more functional-oriented languages, you may want to look up the fold function (also commonly known as reduce) as a generic language built-in replacement for the doWithCustomers function. Filter and map may also help.

8bittree
  • 5,676
  • 3
  • 29
  • 38
38

There are two solutions that seem reasonable:

In practice, we might do this as follows. First, our function contains the business logic of deciding which customer receives how much. Instead of applying that action directly, we return a list of objects representing the action to be taken:

class SendCouponAction:
  customer: Customer
  value: Money

def do_something_sensitive() -> Iterable[SendCouponAction]: for customer in get_all_customers(): if is_in_europe(customer): yield SendCouponAction(customer, Money.euro(1000))

Now normally, we would just perform the actions:

actions = list(do_something_sensitive())

for action in actions: coupon = generate_coupon(action.customer, action.value) coupon.send()

But before doing that, we can log the actions to be taken. For example, we can log the total value:

logger.info(
  "generated %d coupons worth %d in total",
  len(actions),
  sum(coupon.value for coupon in actions),
)

You could now easily make it conditional to actually carry out the action. You can also easily write unit tests for the logic that decides who receives which coupons (though there's still a dependency on the get_all_customers() function that would make tests difficult – its result could be passed as a parameter instead).

Is this a good design? Sometimes.

  • Having a “functional core” is great when possible, because such functions that do not carry out external actions are really easy to test. You don't even have to mock anything, you just provide inputs and look at the outputs.

  • But this can get problematic if carrying out the action is the more interesting part – you might have a core that is essentially empty. For example, I don't think this would be worth it for a basic CRUD application that doesn't really contain any business logic.

  • Trying to achieve such an architecture also gets really tricky if the business logic requires lots of back and forth communication with a data source. You could extract the purely functional parts, but that would end up with a tortured encoding of a state machine that obfuscates the actual business logic.

amon
  • 135,795
8

I would break the operation into two. As long as it's a single operation, regardless of how you program it, there is a chance you run it live instead of dry by mistake.

Conceptually with your coupon example:

  1. produce a list of coupons, save to a database.
  2. send the tokens out to customers

Now I can run and rerun 1 all day and no money is paid out. I can check the result for accuracy, have manual intervention, etc.

When I'm happy, I can perform 2, which I'm careful to program with as little logic as possible and all the checks and safeguards I can think of.

Because I have the set of fully generated coupons, you can put in a check, such as "if total > £1,000,000 don't send".

Ewan
  • 83,178
1

You have coupled the definition of your target group with the action being performed. Instead you can have a function which produces a list or iterator of customers, and a separate function that performs the action:

function getTargetCustomers() {
  targetCustomers = []
  customers = getAllCustomers()
    for each customer in customers
        if customer is from Europe
          targetCustomers += customer
  return targetCustomers
}

function doSomeThingSensitive() { customers = getTargetCustomers() print 'we will pay ' + customers.size() * 1000 for each customer: Give them a 1000 euro giveaway coupon }

MikeFHay
  • 504
0

I'd have merged the dry run code into the sensitive function, and use a Boolean switch to verify if you were just doing a logging counter, or if you were going live - because currently you're one "Dry Run where the called function is the one that isn't called Dry Run" away from executing the main logic, and that seems more dangerous than just decoupling.

You could use a Boolean parameter, but I've also seen a more static variable/global variable context been used in projects as well - showcasing the static private class variable context, I would go with:

dryRun = true; //True if not wanting to run sensitive component, false if sensitive component is fine.

function doSomeThingSensitive() { counter = 0 customers = getAllCustomers() for each customer in customers if customer is from Europe counter++ if dryRun is true log "We would be logging this customer!" else Give them a 1000 euro giveaway coupon if dryRun is true log 'we will pay' + counter * 1000 else log 'we actually did pay' + counter * 1000

Depending on your setup, you may want to make dryRun be a variable you pass in via the function parameter, or as a configuration value in your environment, but you'd want it to be that the default state is to Dry Run the code in your logic, but only as a check against actually sensitive actions, thus keeping your logic mostly the same regardless of if it's a Dry Run or a Sensitive Action Run; if it's a Dry Run when it needs to be a Sensitive Run, you can always make a quick change and then run it again.