Archive for the ‘Coding’ Category

Disagreeing with Code Monkeys

Thursday, November 1st, 2012

attitude.jpg

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.

More Salesforce APEX Code

Thursday, November 1st, 2012

Salesforce.com

Based on a few rules we're trying to implement in the system, I needed to dive back into the Salesforce APEX code for our endpoint(s) and add a few things that weren't there now. Specifically, I needed to take one more step to getting the two endpoints that deal with Merchants a little bit closer together. This wasn't all that easy as APEX is like Java, but not, and testing the code is not easy, and even editing the code isn't all that easy.

Salesforce.com is a web site. So to edit the classes, you need to copy it out of a text field, paste it into some local editor, do the changes, copy that out and paste it back into the text field (assuming it hasn't timed out), and then save it. To test it you need to go to a different web page and put in your URL, with parameters, and then see what results you get.

It's workable, but its nowhere near "efficient". I know from a friend that works there that things are changing, and that would be great. Something to make the development process with Salesforce easier would go a long way to making the entire experience, and therefore the impression about Salesforce, a lot better.

Still… it's possible to write code, and test it. But in a group, it means that you have to be very good with communication as you can easily step over people's changes if you aren't careful. And if you have someone changing the mapping code while you're trying to change what you get out of the API, then it's going to get ugly.

And it did.

Very.

It was a pretty stressful time, but in the end, I got what I needed done. Just not very happy with the overall experience. Certainly something to be avoided in the future, if at all possible.

Working with Bash is Really Quite Fun

Thursday, November 1st, 2012

Ubuntu Tux

I know the Code Monkeys hate it, but bash is a really great little tool. It's on every box, it's got all kinds of capabilities, and it's simple to use. Now it's a slightly different syntax than Ruby, so I'm sure that has something to do with it's distaste with the kids, but that's their issue, not mine.

This morning I was having a little fun creating summary scripts for the two other processes we run as part of the nightly batch runs. They are also ruby processes, but the other guys haven't taken the time -- OK, who am I kidding? I wrote the only summary script there is, and I just hadn't gotten around to writing them for the other two processes.

One other teammate talked about doing it, but I know he'd have written it in ruby, and then tossed out what I had, and I wasn't about to let that happen. So I took the existing code and simply added parsers for the three files, and allowed the script to tell - based on the file name or a "forcing argument", which of the three parsers to use.

Pretty nice. Very clean, and it's one simple file.

It's probably not 100% complete at this point, but it's a great start, and it's able to point out when things have gone really wrong.

Doing timing comparisons for Salesforce

Wednesday, October 31st, 2012

Salesforce.com

This morning I need to do some simple timing comparisons for the Salesforce Team here at The Shop so they can push this back to the Salesforce.com guys and ask what's going on. I'm taking the overnight runs of Production and UAT hitting the production and staging sandboxes, respectively, and comparing that to running the UAT hardware against the new API sandbox, and simply comparing times for actual runs.

Hardware Sandbox Time
Production (EC2) Production 20:03
UAT (EC2) Staging 20:03
UAT (EC2) API 29:07

At the same time, I'm gathering some DEBUG logging information for NewRelic as their newrelic-rpm 3.5.0.1 Gem is not sending back to them the method call instrumentation data in jruby-1.7.0 that it is in jruby-1.6.7.2. I know this because our NewRelic graphs go silent the same instant we moved from 1.6.7.2 to 1.7.0. However, we do get the CPU usage and the memory usage and even the deployment markers - so I know we're sending something, but it's just not sending the right instrumentation data.

So they asked me to send them a DEBUG log, which seems really reasonable. So I'm getting it as I'm doing the final UAT-to-API timing test.

UPDATE: they have sped up API significantly from yesterday. Whatever they did - it's working and that's OK with me.

Google Chrome dev 24.0.1312.1 is Out

Wednesday, October 31st, 2012

This morning I noticed that Google Chrome dev 24.0.1213.1 was out and the big change appears to be in the new WebKit (537.17) and a bunch of fixes related to the bookmark syncing and stability issues. I have to say that I really like the new look of the blog as it's much cleaner than the old, and it's also nice they are moving along with WebKit updates.

Good news.

More Rubyisms that are Back to Haunt Us

Tuesday, October 30th, 2012

Code Monkeys

I really don't like the "shortcuts" a lot of people put into code. They don't really make it more readable, and they certainly make it a lot more brittle. Case in point, today I realized that the code in a rule was written:

  if days_since_assignment && days_since_activity
    # do something
  end

where these variables/methods are nil for those times when there is no respective date. This seems OK, but it's really not. In order to do a good, robust, check, we need to see if it's not nil and then check the value or include it in an inequality. There's no other way.

Now your typical Ruby Code Monkey will say "No! That's not the Ruby Way!" and they'd be right, and the Ruby Way is, in this instance, horrible for so many reasons.

I'm a fan of making things comparable wherever possible, and that means get rid of the nils. Make it simpler - use Float::INFINITY for the return value of a "days since" method if the date doesn't exist. At least that way, it makes sense in all comparisons, and it's at least no worse in some edge cases for testing values. Most often times, it's exactly what you want.

So I had to go in and change what appeared to be "working" code to something that actually worked as intended all because of this desire to minimize typing. Had the original author said:

  if !days_since_assignment.nil? && !days_since_activity.nil?
    # do something
  end

then at least it would be clear what was going on. Returning an INFINITY now makes it clear that we're never going to fail these checks as a nil is never a possible value.

It's all this maturity that I see missing: no comments… minimal typing… over the top refactoring… fascination with new and shiny things… I believe these guys could be great, but they limit themselves and they don't have to. They can see what they should be doing, and they even know they should be doing it. But they choose not to.

I wish they didn't make that choice.

Horrible Performance from Salesforce

Tuesday, October 30th, 2012

This morning I come in to see that we're horribly behind in processing the UAT divisions! The new Salesforce sandbox is a joke! I've got to trim back what we're doing in UAT and get the Salesforce Team on this because we can't let this go on… it's simply unusable.

UPDATE: it took 16 hours to finish this run! That's up from about 3.5 hrs last night - all due to the new Salesforce sandbox. This can't stay this way… I'm moving UAT back to the Staging sandbox as it had decent performance.

Moved to Another New Salesforce Sandbox

Monday, October 29th, 2012

This afternoon I had to move our development and UAT environments to a new Salesforce sandbox. This one is supposed to be the place we'll be doing our development and releasing code to the Salesforce Team's promotion process, so it made sense to get it right and just do it.

The process wasn't hard - I've got it all documented from the last time I had to do this, so it went pretty smoothly. Just needed to get it done.

Do it Right – There is No Trade-Off

Monday, October 29th, 2012

bug.gif

I've been thinking a lot about a conversation I had with my manager about producing versus well thought-out design. He seemed to think that it was a trade-off, and the more I think about it, I think he's wrong. Oh sure, there are tons of trade-offs in engineering and software development, but there isn't a trade-off in putting out good code every day, and doing it "right". There can't be.

If I'm putting out a bunch of features in a day - every day, then they can't be hacks, or some day they are going to catch up with me and make it impossible to go forward. That's not sustainable. What is sustainable is to make sure that you're writing good code… well thought out code… but you're doing it like you mean it.

For example, today I had to go through a lot of code written by one of the Code Monkeys that was really slapped together and included no documentation at all. It was a rules system and it wasn't working cleanly because the methods were quite the rules, and they weren't quite the factors of the rules, so there were nasty dependencies, etc. that made it very hard to figure out what was happening and how to fix it.

I had to clean it all up, make the methods either condition checks or rules, and all rules simply were collections of condition checks, had to clean up some of the tests based on the logging strings, and in general, just make it a lot cleaner.

This didn't take me long, and it took me a lot less time - even under pressure of fixing bugs, then it did to create it by the original author. So it's not about the time required, it's the actions that are taken.

There's no trade-off here. You have to be good at what you're doing or you're going to make a mess. Period.

Fixing Bugs Placed by Careless Coders

Friday, October 26th, 2012

bug.gif

I can see when code has been lazily copy-n-pasted and not really incorporated properly. I can see when coders leave in hard-coded values when they thought they were changing things out. It's not all that hard. This morning I've been doing quite a bit of this kind of bug fixing. It's not all that hard - it's pretty easy stuff to spot, but it makes me sad for the guy that wrote it, because I'm not going to see him in the same light as I had.

Also, comments would be nice when I'm digging in a new web framework (Sinatra), and I really need to get up to speed on these Ruby web systems sooner rather than later as it's going to become more important to me that I'm able to write these - as opposed to just bug fixing them.

I suspect they just don't care, and aren't motivated in the least to care. So I really have to try very hard not to think it's who they are - just how they are responding to the job.