Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse formatted number string to number #41

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
34 changes: 31 additions & 3 deletions src/FormatNumber.elm
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
module FormatNumber exposing (format)
module FormatNumber exposing
( format
, parse
)
cuducos marked this conversation as resolved.
Show resolved Hide resolved

{-| This simple package formats `Float` numbers as pretty strings. It is
flexible enough to deal with different number of decimals, different thousand
Expand All @@ -22,7 +25,7 @@ Just convert them to `Float` before passing them to `format`:
-}

import FormatNumber.Locales as Locales
import FormatNumber.Parser exposing (parse)
import FormatNumber.Parser as Parser
import FormatNumber.Stringfy exposing (stringfy)


Expand Down Expand Up @@ -139,5 +142,30 @@ import FormatNumber.Stringfy exposing (stringfy)
format : Locales.Locale -> Float -> String
format locale number_ =
number_
|> parse locale
|> Parser.parse locale
|> stringfy locale


{-| Parses a pretty string into `Maybe Float`:

import FormatNumber.Locales exposing (Decimals(..), Locale, base, frenchLocale, spanishLocale, usLocale)

parse { base | thousandSeparator = "," } "100,000.345"
--> Just 100000.345

parse { base | thousandSeparator = ",", negativePrefix = "-" } "-100#000"
--> Just -100000
Comment on lines +156 to +157
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the # in the -100#000 example? If # is not a thousand separators, this example seems wrong, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It is wrong. But, this is the problem with current approach of cleaning integer part of non-digits. Thousand separator does not have any value in parsing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can opt for a strategy to validate every every non-digit is one of the expected tokens (negative prefix or suffix, zero prefix or suffix, separators):

  1. Split the value by the regex \d+
  2. Check if the resulting list contains only expected tokens


parse { base | negativePrefix = "", negativeSuffix = "-" } "100,000-"
--> Just -100000

parse { base | negativePrefix = " ~", negativeSuffix = "" } " ~100,000"
--> Just -100000

parse { base | thousandSeparator = ",", negativePrefix = "(", negativeSuffix = ")" } "(100,000.546)"
--> Just -100000.546

-}
parse : Locales.Locale -> String -> Maybe Float
parse locale str =
Parser.parseString locale str
60 changes: 60 additions & 0 deletions src/FormatNumber/Parser.elm
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ module FormatNumber.Parser exposing
, addZerosToFit
, classify
, parse
, parseString
, removeZeros
, splitInParts
, splitThousands
)

import Char
import FormatNumber.Locales exposing (Decimals(..), Locale)
import Regex
import Round
import String

Expand Down Expand Up @@ -378,3 +380,61 @@ parse locale original =
| prefix = locale.zeroPrefix
, suffix = locale.zeroSuffix
}


{-| Given a `Locale` parses a `String` into a `Maybe Float`
-}
Comment on lines +446 to +447
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some examples here, both for documentation and for tests ; )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason I did not add any tests here is because parse function in FormatNumber.elm is just calls this function and does nothing else. I thought it would be repetition.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse function in FormatNumber.elm is just calls this function

So, what about moving this function to FormatNumber.elm?

parseString : Locale -> String -> Maybe Float
parseString locale value =
let
isNegative : Bool
isNegative =
negativePrefixMatch && negativeSuffixMatch

negativePrefixMatch : Bool
negativePrefixMatch =
value
|> String.startsWith locale.negativePrefix

negativeSuffixMatch : Bool
negativeSuffixMatch =
value
|> String.endsWith locale.negativeSuffix
praveentiru marked this conversation as resolved.
Show resolved Hide resolved

chrs : Regex.Regex
chrs =
"[^\\d]"
|> Regex.fromString
|> Maybe.withDefault Regex.never

splitValue : String -> List String
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we should use String -> (String, String) here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I change this, then I can move entire logic of joinParts into the parseString. I like the current structure as it cleanly segregates that out of parseString and allow me to test it separately.

Let me know if you still want it changed. I will do it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think making impossible states impossible worth the job : )

splitValue nbr =
nbr
|> String.split locale.decimalSeparator
|> List.map (Regex.replace chrs (\_ -> ""))
praveentiru marked this conversation as resolved.
Show resolved Hide resolved

sumNumber : Float -> Float -> Float -> Float
sumNumber i d pow =
if isNegative then
-1 * (i + d / (10 ^ pow))

else
i + d / (10 ^ pow)
praveentiru marked this conversation as resolved.
Show resolved Hide resolved
in
case splitValue value of
praveentiru marked this conversation as resolved.
Show resolved Hide resolved
integer :: decimal :: [] ->
Maybe.map3 sumNumber (String.toFloat integer) (String.toFloat decimal) (Just (toFloat (String.length decimal)))

integer :: [] ->
Maybe.map
(\n ->
if isNegative then
-1 * n

else
n
)
(String.toFloat integer)

_ ->
Nothing