Scott Eade wrote:
I have had a look at this and have the following comments:
Thank you for your review.
1. I think NumberFormat.parse() will convert "12.3foo" to 12.3 where
as the old Double.parseDouble() would throw a
NumberFormatException. I don't think we should be blindly dropping
anything after the number so I think we need to detect this and
throw an exception. This also applies to the corresponding Float
and BigDecimal methods.
That would involve parsing with ParsePosition, right?
2. Why doesn't getDoubleObject() just invoke getDouble()?
getDoubleObjects() use getDoubles()? Being pedantic, getDouble()
and getDoubles() have a common block of code that could be in a
private method. This would apply to many of the other methods
also. Whether or not these should be addressed is a matter of
style - clearly they were like this before your changes.
No, they weren't. I just checked. Actually I changed very little in the
logic of the methods. But yes, you are right, that'd be less code and
less code is a Good Thing (TM). As numberFormat.parse() returns a
Number, it would be even possible to share the code between Float,
float, Double, and double. I'll try to fix this over the weekend.
Try adding the following to BaseValueParserTest.testGetDouble():
parser.add("unparsable2", "1a");
result = parser.getDouble("unparsable2");
assertEquals(result, 0, 0);
Will do.
The TurbineRunDataService change looks okay, but I will try it out
shortly to confirm that I can easily override the locale on a per user
basis.
That's exactly my problem, too. As I see it, the Turbine request
processing is missing a call to data.setLocale() at some (early) place.
I'd propose to add a method to the LocalizationService, something like
setRunDataLocale(RunData) which can be overridden in the implementation.
Bye, Thomas.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
