search

myob-oss / AccountRight_Live_API_.Net_SDK

This is the repo for the MYOB AccountRight Live .Net SDK
MIT License
23 stars 43 forks source link

Type mismatch - TaxCode.Rate #41

Open jadinning opened 8 years ago

jadinning commented 8 years ago

I am not sure if you consider this an issue (it can be bypassed with a typecast) but TaxCode.Rate in the API is a Decimal (8,4) in the API but declared as a double in the SDK

Regards, John.

sawilde commented 8 years ago

The decimal declaration in the docs refers to the database storage type; which I think needs looking at - @devjack what do you think?

TaxCode rates up to 4 decimal places can be correctly represented in both decimal and double.

devjack commented 8 years ago

It's more important to match the API data type since this is from a development standpoint the authoritative data source (any internal data types would need to be mapped to the public JSON anyway). If we can change it in a non BC breaking manner then I think its beneficial for consistency; but definitely not a critical change. Easily rolled into a future release.

sawilde commented 8 years ago

@devjack - I am more thinking that the database schema type shouldn't be used in the docs - the only useful info from an API view is should be string(+min/max length), int/long or double/decimal(+significant digits). The database schema does reflect the above but if you don't know SQL it can be a bit confusing.

jadinning commented 8 years ago

@sawilde - that makes sense to me, but actually I would like to see both, so we can be sure they match. I am taking some data and holding it in a temporary SQLCE database so it is nice to use the same database storage type as used by AccountRight.

However, I had an instance last year (it may have been fixed now) where AccountRight held, and the API returned, a bigger value than could be stored in the type (Decimal (12.3)) specified in the doc. This may well have been a documentation error, but if I had known the API data type, I may not have had this overflow issue.

sawilde commented 8 years ago

@jadinning - useful to know