Facing the Way it Is, as Opposed to How You Want it to Be
Today I spent a good bit of time planning out the changes I need to make in this one project I'm taking over. It's not the way I'd build it, but that's really beside the point... it is and I have to work with that. Plain and simple.
So I'm looking at why a strike for an option that should be 1.34 is really 1.34000003674 - I've run into this before in my previous job: rounding in the format change from string to float or double. It's just the way things are. So I needed to see how things were being converted and then see where I might be able to put in code to properly round to something like 1.0e-4, or something.
I found the data in the database was stored as a real, and that's not a good sign right off the bat, as I think that's going to be mapped into a Float by JDBC as opposed to the Double that might have prevented this. But the problems didn't end there. The code I found mapped the value to a String and then later, it was mapped to a Double and then used. By then, I'm not at all surprised that it had the extra digits. Too many conversions. Way too many.
But again, in order to minimize the impact to the code, it was smarter to look at where the options were being created and insert the rounding on the strike at that point. It's not what I'd have wanted to do, but again, I had something that was close to right, and so it wasn't really worth the risk of changing all the mapping and database code when the problem could be addressed very simply in the setter code of the new option:
public synchronized void setStrike(Double value) { _strike = Math.rint(value * 1.0e4)/1.0e4; }
with this, we'll get the strike rounded to the nearest 1/100th of a cent. Easy to do, and it'll wipe out all the "problem digits" that we're seeing.
But I wasn't done yet. There was more code that needed fixing on the strikes. There was a place in the code where they were trying to create the "short-hand name" of the option and it had the following code to display the strike:
double decimal = strike - Math.floor(strike); int strikeInt = new Double(strike).intValue(); ... if (decimal > 0.) shortName.append(strike); else shortName.append(strikeInt);
where the goal is to render the Double 1.5 as "1.5" and the Double 95.0 as "95". Basically, if there's no fractional part, then don't show the decimal point. Seems good, but this is a horrible way to do it. A far better way would be to say:
DecimalFormat fmt = new DecimalFormat("0.####"); ... shortName.append(fmt.format(strike));
where the formatter will properly round to four decimal places and also drop the entire decimal part if there isn't one to display.
Sure, the DecimalFormat isn't thread-safe, but I'm not expecting it to be. Make one, use it, drop it. They are meant to be very lightweight classes - which is why they aren't thread-safe. But in this case, they are perfect for the occasion.
Then I got into another problem that I needed to tackle and realized that I didn't see where the original author had made any allowances for the feature. I looked more and still found nothing. I then shot off an email as I dug even further. By the time I talked to him, I was convinced that he simply didn't have any modeling of this particular business rule in the code. It was all based on configuration files that duplicated these business relationships he didn't want to include.
I talked to him and he fessed up about not making any allowances for this because he thought it was wrong. I pointed out that all he did was to push it into static config files in the server, and he agreed.
Right or wrong, a business rule is a business rule. It's not about "correctness". It's about how the users think of things. If they choose to think of things as "colors" then model the colors - just as they think of them. Don't make the quality judgement on the way things are - just use them.
I had done this with his code - from the get-go, but he wasn't as accommodating of the users. I told him what I needed to do and why, and was very nice about the judgement, but in the end, we need to add in this concept as it's critical to being able to do several things that are globally important, even if they aren't important to him.
It's important to understand that there's a time to correct something because its wrong, and leaving it as-is is going to cause more problems in the future, and when it's time to let it flow over you and work with the imperfect system that you've been given.