Today I was working on a few stories that have to do with a rule system we're putting into the app, and one of the Code Monkeys rolled on over and started to discuss his opinion of what we should do with regards to the design of the rule system. Now bear in mind… we have something, and he's talking about completely changing the flow and logic of that to make it, in his words: "Simpler to understand". Specifically, the existing rules that we coded up were based on a simple doc the program manager wrote up. The current problem was that he had unspoken assumptions in the rules and so he filed several bugs on the rules after they were coded up.
Normally, this would be OK, but I wanted to use this as an educational opportunity for this PM, as I really thought it warranted it. If he can't express the rules completely in english, we're going to have a very hard time coding them up to his satisfaction. So there was plenty of back-and-forth, give-and-take in this conversation.
We talked about how he might organize his thoughts… how me might represent them on paper, all with the goal of being able to get to a point where we had a solid set of rules that we could take and code against.
In the middle of this, over rolled the Code Monkey.
Now I'm only saying that in retrospect. This guy is smart. He's nice. And he can write good code when he chooses, it's that he often finds it's more fun to play games or just not code. In any case, he came over with what he thinks we should be doing.
To frame this properly, we currently have a system where there are several condition-type-methods on the classes, and these are then used in combination in methods to create each "rule". For example, if we wanted to have a rule that said "New sales reps get 10 days to content a merchant or the merchant is reassigned" we might write up:
def check_new_merchant(account)
if !account.new_sales_rep? && account.no_activity_since_assignment?
reason = "No activity on merchant"
end
reason
end
so if we get a nil, then we know it didn't "trip" the rule.
Now there can be tests that say "leave this merchant alone" as well as tests that say "re-assign this merchant", so we have to put them in a block - in a specific order, to make sure that we don't build overly complex rules. Sure, it's non-trivial, but it's a workable system in the same sense that a switch or case statement is. You have to run through it from top to bottom as order matters.
The proposed solution was to have a series of classes - all based on a root rule class. This class would look something like this:
class rule
def applies?(account)
end
def condition?(account)
end
end
where the applies? method is used to see if this rule applies to the Merchant account, and the condition? method is used only if the merchant returns true from applies?.
What's really needed is a tri-state output of the rule: Yes, No, and Don't Care. But short of that, the first method filters out the "Don't Care" states, and then the second handles the other two. Not bad, but it could be better if we had a logical value that could handle this in the language. Ah… in C++ this would be easy…
There's nothing wrong with the approach, but it doesn't really reduce the complexity. If you have a list of these classes, then the order is just as important - unless you make sure that they are, by construction, mutually exclusive. This means that whereas the existing scheme tests one thing after the other, this scheme would require that we include the logic of all tests in each test so that we didn't have a mistaken triggering.
You could make it all work in ruby by adding functionality and methods, but then the runtime becomes very hard to debug, but hey, that's ruby, and that's how you'd do it. But that doesn't really reduce the complexity. It just shifts it.
And there came the disagreement.
My point was that there's really no difference in complexity - just a shifting. But the advantage of the method we have now, that I have been working with, is that we have a fail fast approach. If we have 1000 merchants to test, and 900 of them will fail on the first test, the current approach won't run through all the other tests for those 900 - so it'll be faster.
His point was that he'd trade that for what he believed was a "simpler" system.
Not really bad - yet, but we're getting there fast.
When I got him to agree that it was just a shift, and that there was a performance bonus, his complete unwillingness to "agree to disagree" was the problem. Totally unwilling to agree that I had a point, and that it was equally valid, he simply argued over and over again, until I'd gotten to the point that I was about to say something very unkind. Instead, I said: "Fine. Do it your way", and looked at the PM and said "Talk to him now".
This wasn't OK with the PM. Why? Because I'm the reason things are working. I'm the reason things are delivering. I'm the one that Gets Things Done. He was know for coming in at 10:00 am, and playing a lot of ping pong. So it was clear to him that he wanted me on his issues, and so the conversation was not "a difference of opinions", it was a Code Monkey vs. a Coder, and he didn't like the outcome at all.
Too bad. I'm not the guy's manager. I'm not his boss. I can't make him do anything, and he's already gone into my code after the fact and rewritten parts in the name of "clarification" that gutted things I begged him not to remove. Too bad. I can't stop him, and I'm not going to get into a war of tit-for-tat in git. No way.
So it's one of those classic problems, and I think I have an idea about how to keep it from happening next time: Agree with his point, but say "I've got this - you get the next one" and don't let him dig in his heels. Might not work, but it might, and that's worth a shot.
