[QFJ-300] "BigDecimal" build of QuickFIX/J doesn't preserve precision in quantities Created: 05/Mar/08  Updated: 07/Aug/08  Resolved: 11/Apr/08

Status: Closed
Project: QuickFIX/J
Component/s: Engine
Affects Version/s: 1.3.1
Fix Version/s: 1.3.2

Type: Bug Priority: Default
Reporter: Graham Miller Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: None
Environment:

ant -Dgenerator.decimal=true

Attachments: Text File DecimalFieldTest.java     Text File qfj-300.patch    

 Description   

When you use a "BigDecimal" build of QuickFIX/J, it should preserve the scale of the number inside a message. For example, if I set the Price field to be "10.3000" I should get that same value (with the trailing zeroes) back from fieldMap.getDecimal(), and similar calls. Included is a simple test.

	public void testQuickFIXAssumptions() throws FieldNotFound, InvalidMessage{
		BigDecimal originalPrice = new BigDecimal("10.3000");
		assertEquals(4, originalPrice.scale());
		Message message = new Message();
		message.setField(new Price(new BigDecimal("10.3000")));
		BigDecimal extractedPrice = message.getDecimal(Price.FIELD);
		assertEquals(4, extractedPrice.scale());
		assertEquals(new BigDecimal("10.3000"), extractedPrice);
		String newOrderString = message.toString();
		Message rehydratedMessage = new Message(newOrderString);
		BigDecimal rehydratedPrice = rehydratedMessage.getDecimal(Price.FIELD);
		assertEquals(new BigDecimal("10.3000"), rehydratedPrice);
		assertEquals(4, rehydratedPrice.scale());
	}


 Comments   
Comment by Toli Kuznets [ 06/Mar/08 ]

So it seems that the culprit is the DecimalConverter.convert(BigDecimal) function.
In the current implementation it's calling through to the DecimalConverter.convert() method that takes the padding like this:
public static String convert(BigDecimal d)

{ return convert(d, 0); }

Underneath that goes into the DoubleConverter.getDecimalFormat(padding).format(d) and causes the incoming BigDecimal to lose its scale since the "padding" (or scale) is set to 0.

The way to fix that would be to use call either send in the d.scale() (which is not the best way since scale may be negative for big decimals) or rewrite the function as such:
public static String convert(BigDecimal d) { return convert(d, 0); }

Ideally, I'd vote to remove the function with "padding" altogether, and just rely on the scale() method in BigDecimal instead.

thoughts?

Comment by Toli Kuznets [ 06/Mar/08 ]

complete unit test for this issue.
Works when you patch DecimalConverter to have this convert() function:

public static String convert(BigDecimal d)

{ return d.toPlainString(); }
Comment by Graham Miller [ 07/Mar/08 ]

That unit test will only compile against a "BigDecimal" build, right? Because it uses a constructor like new Price(new BigDecimal("10.3000"))? If that's the case, we probably can't add it to the core unit tests.

Comment by Toli Kuznets [ 10/Mar/08 ]

Patch with the fix for the bug, and fixes to the sample applications to handle both doubles and BigDecimals by switching internal qty handling to BigDecimals.
Added a unit test that verifies the bug is working if fields are built with BigDecimal support.

Comment by Steve Bate [ 11/Apr/08 ]

I've applied the patches. For some reason, there were a few places where the patch wasn't performed automatically. I think I've applied the patch correctly but can you (Toli) try this out with the big decimal build? Thanks.

Comment by Toli Kuznets [ 06/May/08 ]

Steve, i've made some additional changes to the examples files which weren't part of your changes and checked them in http://quickfixj.svn.sourceforge.net/viewvc/quickfixj?view=rev&revision=804

The examples now work for both regular doubles and BigDecimal builds

Generated at Mon May 13 13:59:58 UTC 2024 using JIRA 7.5.2#75007-sha1:9f5725bb824792b3230a5d8716f0c13e296a3cae.