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

money notation #95

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

money notation #95

wants to merge 4 commits into from

Conversation

lfaoro
Copy link

@lfaoro lfaoro commented Aug 29, 2021

fixes #94

I had to add a .gitignore file as the editor .idea/.vscode directories were being picked up. Took the liberty to add the other standard golang gitignore entries as well, hope you don't mind.

lfaoro added 2 commits August 28, 2021 16:38
Signed-off-by: Leonardo <lfaoro@gmail.com>
Signed-off-by: Leonardo <lfaoro@gmail.com>
.gitignore Outdated
Comment on lines 8 to 12
# Test binary, built with `go test -c`
*.test

# Output of the go coverage tool, specifically when used with LiteIDE
*.out

Choose a reason for hiding this comment

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

My own two cents:

If you need to ignore something that is specific to a project it belongs in the project's .gitignore

If you need to ignore something that is specific to the developer it belongs in the developer's global .gitignore.

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

I'll remove the .gitignore from the PR

"fmt"
)

func Finance(f float64) string {

Choose a reason for hiding this comment

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

I wonder if it would make sense to take some optional Options which could set things like which $ sign to use and if it should be prepended or appended (or omitted entirely) and could control the # of decimal points.

So you could do things like:

  1. 1.83M USD
  2. €2.34T
  3. 48.9945

etc. Could also look at the various locales used with https://www.php.net/manual/en/function.money-format.php

Copy link
Author

@lfaoro lfaoro Sep 27, 2021

Choose a reason for hiding this comment

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

Very good idea, I'll add the Options pattern, the options will be:

  • WithSign(s string) // can pass "" to remove the default ($) sign
  • WithAppend() // if no append, Prepend is default -- otherwise could add both options
  • WithPrecision(n int) // defaults to .2

WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

lfaoro added 2 commits October 8, 2021 13:40
This reverts commit 8e799e8.
Signed-off-by: Leonardo Faoro <lfaoro@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

money notation
2 participants