byteverse / scientific-notation

Numbers in Scientific Notation
Other
4 stars 2 forks source link

encodeFixed? #5

Open mwotton opened 1 year ago

mwotton commented 1 year ago

Would you accept a patch that adds a function that always encodes in a fixed decimal format? I'm using your code to process some JSON into CSV and would prefer a simple & predictable numeric encoding format, even if it's pretty bloaty.

(Thanks for the cool software, btw, json-syntax is way faster than Aeson for my workload.)

andrewthad commented 1 year ago

I believe that you're talking about a function that looks like this:

encodeFixed ::
     Int -- ^ Number of digits to include after the period
  -> Scientific -- ^ Number to encode
  -> ShortText

If we do something like:

encodeFixed 3 (small 5 0)

Would the output be 5 or 5.000? I think it would be the second one, but it would be good to be precise in the function's documentation. And I think it would have truncating (rather than rounding) behavior for numbers that could not be represented exactly. Alternatively, you might want a function that's more like this:

encodeDecimal ::
     Scientific -- ^ Number to encode
  -> ShortText

Which just always does the maximum number of decimal places. This second one is actually pretty easy to write since builderUtf8 currently has a special case for exponents between -50 and +50:

builderUtf8 :: Scientific -> Builder
builderUtf8 (Scientific coeff e big)
  | e == 0 = Builder.intDec coeff
  | e == minBound = let LargeScientific coeff' e' = big in
      if | coeff' == 0 -> Builder.ascii '0'
         | e' == 0 -> Builder.integerDec coeff'
         | e' > 0 && e' < 50 ->
             -- TODO: Add a replicate function to builder to improve this.
             Builder.integerDec coeff' <> Builder.bytes (Bytes.replicate (fromInteger e') 0x30)
         | e' < 0, e' > (-50), coeff' > 0, coeff' < 18446744073709551616 ->
             let coeff'' = fromInteger coeff' :: Word
                 e'' = fromInteger e' :: Int
              in Builder.bytes (encodePosCoeffNegExp coeff'' e'')
  ...

So to implement encodeDecimal, all that is needed is to copy builderUtf8 and make the [-50,+50] "special case" the only code path.

I'd take either of these in a PR as long as some relevant tests are added to the test suite.

mwotton commented 1 year ago

thanks! I think the second is closer to what I want, I'll get a PR together.