Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Chris Sosnin <k.sosnin@tarantool.org>,
	tarantool-patches@dev.tarantool.org, sergepetrenko@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 2/2] Add IsInt method for checking the fractional part of a number
Date: Thu, 25 Jun 2020 00:23:12 +0200	[thread overview]
Message-ID: <5ae054f7-2472-a80c-1b3d-1f89ae40f21a@tarantool.org> (raw)
In-Reply-To: <f575228474c4c0e96043b2f3e29da66632aaaa52.1593017012.git.k.sosnin@tarantool.org>

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().

> +  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".

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

  reply	other threads:[~2020-06-24 22:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 16:53 [Tarantool-patches] [PATCH v2 0/2] decNumber utilites for SQL Chris Sosnin
2020-06-24 16:53 ` [Tarantool-patches] [PATCH v2 1/2] Refactor decNumberFromString Chris Sosnin
2020-06-24 22:22   ` Vladislav Shpilevoy
2020-06-25 14:21     ` Chris Sosnin
2020-06-24 16:53 ` [Tarantool-patches] [PATCH v2 2/2] Add IsInt method for checking the fractional part of a number Chris Sosnin
2020-06-24 22:23   ` Vladislav Shpilevoy [this message]
2020-06-25 14:22     ` Chris Sosnin
2020-06-25 21:04 ` [Tarantool-patches] [PATCH v2 0/2] decNumber utilites for SQL Vladislav Shpilevoy
2020-06-26 10:57 ` Serge Petrenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5ae054f7-2472-a80c-1b3d-1f89ae40f21a@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=k.sosnin@tarantool.org \
    --cc=sergepetrenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 2/2] Add IsInt method for checking the fractional part of a number' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox