node-rfc: Possible wrong handling of RFCTYPE_BCD in recently introduced fill type checks

should be back again in just published 1.0.2

Fixed by ac6eb8883b95d7f9aae3c22440578d95edc6ab51, will be provided in next release.

Please check the prerelease 6, just published and these unit tests

Does this mean, that the return type of BCD values is now again a float instead of a string or am I misunderstanding you?

Regardless if BCD values returned as strings or floats, from ABAP to nodejs, any follow-up calculations in nodejs would require a conversion to Decimal.js or similar object, to prevent rounding issues:

> x =0.1 # received as float from ABAP, or as a string '0.1', and in nodejs parsed as float
0.1
> x.toPrecision(55)
'0.1000000000000000055511151231257827021181583404541015625'
> y = 0
0
> for (let i = 0; i < 10000000; i++) y+= x;
999999.9998389754

with Decimal

> D = require('Decimal.js')
> x = D(.1)
> y = D(0)
> for (let i = 0; i < 10000000; i++) y = y.add(x);
> y.toNumber().toPrecision(55)
'1000000.000000000000000000000000000000000000000000000000'
>

In this context, returning ABAP BCD as node-rfc string or float, I see both equally good for the representation and both equally wrong for calculations and more the question of convention and convenience for one or another application?

Do you see it the same way?

The optimal solution would be perhaps a node-rfc configuration option, to provide a pointer to JS Decimal function, for ABAP BCD values conversion. Without configuration, which is standard only because no standard decimal exists in JavaScript, use the string (not recommended standard), providing options to use the float (not recommended option), or decimal object (recommended option). Would that work for all?

Reading a BCD value, coming from MD_STOCK_REQUIREMENTS_LIST_API BAPI for example, starts with reading it as a string, from SAP NW RFC data container. That is the only rounding-safe way to get this value from ABAP:

    case RFCTYPE_BCD:
    {
        // An upper bound for the length of the _string representation_
        // of the BCD is given by (2*cLen)-1 (each digit is encoded in 4bit,
        // the first 4 bit are reserved for the sign)
        // Furthermore, a sign char, a decimal separator char may be present
        // => (2*cLen)+1
        unsigned int resultLen;
        unsigned int strLen = 2 * cLen + 1;
        SAP_UC *sapuc = mallocU(strLen + 1);
        rc = RfcGetString(functionHandle, cName, sapuc, strLen + 1, &resultLen, &errorInfo);
        if (rc != RFC_OK)
        {
            free(sapuc);
            break;
        }
        resultValue = wrapString(sapuc, resultLen);
        free(sapuc);
        break;
    }

At this point, at C++ and SAP NW RFC SDK Library level, only very restricted subset of node JS methods is provided by node addon api, we could use to parse this string. The length of input JavaScript string for example is at this level not any more available, type checks are restricted and extending node addon api, by parseFloat for example, would take a while, if accepted at all (see Check if Napi::Value is integer number? or More special types check for example).

Python RFC wrapper does not have this limitation and returns Python Decimal object because Cython makes practically all Python functionality available here.

What is the biggest practical problem with current node-rfc behaviour, accepting string, number or Decimal object as BCD input and returning only string as output? Parsing on application side is not slower and not always required. For web applications’ CRUD cycles for example, the number or string format does not not matter, at least in my web projects with node-rfc. For queries/calculations, stored procedures and ABAP make more sense anyway, although the need for this can’t be completely excluded on app server or frontend layer.

What is the biggest pain point in your scenario/application ?

I’m far from being an expert in data representation formats, but let’s approach this from a different angle: IMHO a library like node-rfc is supposed to be more correct than your own applications because it’s potentially used by quite a lot of people, who will be very confused if it causes some hard-to-find/debug rounding errors. Your examples might work for you, and I admit that probably most calculations with money floats will never cause any trouble in the real world. But still, I believe a library has to be more precise, even if this comes at the cost of being more unwieldy.

If you look at what a BCD represents and how it represents it, we can see that it uses a fixed number of bits to represent all the numbers of a value. So there’s no potential for rounding errors or imprecise representations of a given number.

Floating point, on the other hand, uses an approximation of a value with a potentially infinite number of decimal digits.

So IMHO, the closest built-in data type to represent BCD values in Javascript is in fact a string. That does not necessarily mean that we should immediately stop thinking about whether it would make sense to have node-rfc return BCD values as float optionally.

Don’t get me wrong: I hate nothing more than academic discussions about practical problems, but in this case, I firmly believe that the fix (which reverted a breaking change) was the right thing to do.

What do you think, @bsrdjan?

Nice theoretical example … unfortunately none from practice. A simple parseFloat of strings would throw out the same “error”. It is a pity that you have such an effort now because of old well-known Javascript properties. If the mistake had disturbed me at one point, I would have written

parseFloat ((1.04 - 0.43) .toPrecision (3))

0.61 … voila

Here are some examples. Also with decimal.js:

“Maybe there’s a way to grab the SAP field definition and come up with some automated way to convert BCD values to floats?”

Can it be more complicated, please?

Better way:

Maybe there’s a way to grab the SAP field definition and come up with some automated way to convert BCD values to strings?