Re: [Tarantool-patches] [PATCH v2 2/2] Add IsInt method for checking the fractional part of a number

[Chris Sosnin <k.sosnin@tarantool.org>]

Thank you for the review!

> On 25 Jun 2020, at 01:23, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Thanks for the patch!
> 
> Technically looks fine. But see two style comments below.
> 
> On 24/06/2020 18:53, Chris Sosnin wrote:
>> Currently there is no efficient way to do this.
>> 
>> Needed for tarantool/tarantool#4415
>> ---
>> decNumber.c | 27 +++++++++++++++++++++++++++
>> decNumber.h |  1 +
>> 2 files changed, 28 insertions(+)
>> 
>> diff --git a/decNumber.c b/decNumber.c
>> index e248656..26acec3 100644
>> --- a/decNumber.c
>> +++ b/decNumber.c
>> @@ -501,6 +501,33 @@ uLong decNumberToUInt64(const decNumber *dn, decContext *set) {
>>   return 0;
>>   }  // decNumberToUInt64
>> 
>> +Flag decNumberIsInt(const decNumber *dn) {
> 
> 1. It seems Flag type is internal. For public API flags the lib uses int32_t.
> For example, look at decNumberIsNormal(), decNumberIsSubnormal().

I agree, Flag is only used inside decNumber.c, I looked at the wrong function.
Fixed.

> 
>> +  const Unit *up=dn->lsu;
>> +  if (dn->exponent>=0) {
>> +    return 1;
>> +    }
>> +   else {
>> +    Int count=-dn->exponent;
>> +    // spin up whole units until reach the Unit with the unit digit
>> +    for (; count>=DECDPUN; up++) {
>> +      if (*up!=0) return 0;
>> +      count-=DECDPUN;
>> +      }
>> +    if (count==0) return 1;             // [a multiple of DECDPUN]
>> +     else {                             // [not multiple of DECDPUN]
>> +      Int rem;                          // work
>> +      // slice off fraction digits and check for non-zero
>> +      #if DECDPUN<=4
>> +        rem=*up-QUOT10(*up, count)*powers[count];
>> +      #else
>> +        rem=*up%powers[count];          // slice off discards
>> +      #endif
>> +      if (rem!=0) return 0;
>> +      }
>> +    }
>> +    return 1;
>> +  }  // decNumberIsInt
>> +
>> /* ------------------------------------------------------------------ */
>> /* to-scientific-string -- conversion to numeric string               */
>> /* to-engineering-string -- conversion to numeric string              */
>> diff --git a/decNumber.h b/decNumber.h
>> index ffaa3d8..9d3a7e1 100644
>> --- a/decNumber.h
>> +++ b/decNumber.h
>> @@ -169,6 +169,7 @@
>>   decNumber  * decNumberTrim(decNumber *);
>>   const char * decNumberVersion(void);
>>   decNumber  * decNumberZero(decNumber *);
>> +  uint8_t      decNumberIsInt(const decNumber *dn);
> 
> 2. Better move this to the other Is functions a few lines below. Where
> the comment says "Functions for testing decNumbers".

Fixed.

> 
>> 
>>   /* Functions for testing decNumbers (normality depends on context)  */
>>   int32_t decNumberIsNormal(const decNumber *, decContext *);