-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add a way to register custom currencies #41
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
package currency | ||
|
||
import ( | ||
"fmt" | ||
) | ||
|
||
// EmptyCurrencyCodeError indicates that the currency code was empty. | ||
type EmptyCurrencyCodeError struct{} | ||
|
||
func (e EmptyCurrencyCodeError) Error() string { | ||
return "register currency error: empty currency code" | ||
} | ||
|
||
// CurrencyAlreadyExistsError indicates that the currency code already exists in the ISO list. | ||
type CurrencyAlreadyExistsError struct { | ||
Code string | ||
} | ||
|
||
func (e CurrencyAlreadyExistsError) Error() string { | ||
return fmt.Sprintf("register currency error: code %q already exists in ISO list", e.Code) | ||
} | ||
|
||
// RegisterCurrencyOptions defines parameters for registering a new or custom currency. | ||
type RegisterCurrencyOptions struct { | ||
// NumericCode is usually a three-digit code, for example "999". | ||
NumericCode string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also need a CountryCode here (first in the struct, above NumericCode), to provide data for |
||
|
||
// Digits is the number of decimal fraction digits. | ||
Digits uint8 | ||
|
||
// SymbolData is a list of possible symbols and the locales | ||
// in which each symbol is used. | ||
// | ||
// Example: | ||
// []SymbolData{ | ||
// {Symbol: "₿", Locales: []string{"en"}}, | ||
// {Symbol: "BTC", Locales: []string{"uk"}}, | ||
// } | ||
SymbolData []SymbolData | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels odd to call this SymbolData. Why not just Symbols? I would expect to see
That would make it match the GetSymbol API:
Having to initialize a currency.Locale is a bit annoying, but that's how all other parts of this package currently accept a locale, so we're locked in. As a bonus, use of the map means you no longer need a separate struct for the symbol data. |
||
} | ||
|
||
// SymbolData describes one symbol and the set of locales | ||
// for which that symbol applies. | ||
type SymbolData struct { | ||
Symbol string | ||
Locales []string | ||
} | ||
|
||
// RegisterCurrency adds a non-ISO currency to the global structures: | ||
// - currencies | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't make sense for public docs to mention internal structures, as they could change at any time. You want to mention what the function does, not how it does it. |
||
// - currencyCodes | ||
// - currencySymbols | ||
// | ||
// It returns an error if the code already exists in the ISO list, or if the code is empty. | ||
func RegisterCurrency(code string, opts RegisterCurrencyOptions) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This func name gives you
|
||
if code == "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the currency code is empty, this func should exit silently, making the operation a no-op. That makes it consistent with other functions in currency.go (such as GetDigits). |
||
return EmptyCurrencyCodeError{} | ||
} | ||
if _, isoExists := currencies[code]; isoExists { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would argue that you don't want to have this check and error. That would allow people to use currency.Register to override existing currencies, e.g. to change their default number of digits, or the default symbols. We had a discussion about digits recently in #37 where this was identified as a valid use case. |
||
return CurrencyAlreadyExistsError{Code: code} | ||
} | ||
|
||
// Insert into the global `currencies` map. | ||
currencies[code] = currencyInfo{ | ||
numericCode: opts.NumericCode, | ||
digits: opts.Digits, | ||
} | ||
|
||
// Also append to currencyCodes, so that GetCurrencyCodes() is aware of it. | ||
currencyCodes = append(currencyCodes, code) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code also needs to modify |
||
|
||
// If SymbolData is provided, insert symbols into currencySymbols. | ||
if len(opts.SymbolData) > 0 { | ||
// Ensure there's a slice for 'code' in currencySymbols. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comments like these two are not helpful, because they say the same thing that the code does. I'd expect code comments to focus on the "why", not the "what". |
||
if _, ok := currencySymbols[code]; !ok { | ||
currencySymbols[code] = []symbolInfo{} | ||
} | ||
// Add each entry from opts.SymbolData. | ||
for _, s := range opts.SymbolData { | ||
currencySymbols[code] = append( | ||
currencySymbols[code], | ||
symbolInfo{ | ||
symbol: s.Symbol, | ||
locales: s.Locales, | ||
}, | ||
) | ||
} | ||
} | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
package currency | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This library uses black box testing, all other tests are in |
||
|
||
import ( | ||
"testing" | ||
) | ||
|
||
func TestRegisterCurrencyBTC(t *testing.T) { | ||
err := RegisterCurrency("BTC", RegisterCurrencyOptions{ | ||
NumericCode: "1000", | ||
Digits: 8, | ||
SymbolData: []SymbolData{ | ||
{ | ||
Symbol: "₿", | ||
Locales: []string{"en"}, | ||
}, | ||
{ | ||
Symbol: "BTC", | ||
Locales: []string{"uk"}, | ||
}, | ||
}, | ||
}) | ||
if err != nil { | ||
t.Errorf("RegisterCurrency returned an error for BTC: %v", err) | ||
} | ||
|
||
if !IsValid("BTC") { | ||
t.Error("Expected 'BTC' to be valid after registration, but IsValid returned false.") | ||
} | ||
|
||
d, ok := GetDigits("BTC") | ||
if !ok { | ||
t.Error("Expected 'BTC' to be found, but GetDigits says not ok.") | ||
} else if d != 8 { | ||
t.Errorf("Expected 'BTC' digits=8, got %d", d) | ||
} | ||
|
||
symEN, _ := GetSymbol("BTC", NewLocale("en")) // "₿" | ||
if symEN != "₿" { | ||
t.Errorf("Expected '₿' for locale 'en', got '%s'", symEN) | ||
} | ||
|
||
symRU, _ := GetSymbol("BTC", NewLocale("uk")) // "BTC" | ||
if symRU != "BTC" { | ||
t.Errorf("Expected 'BTC' for locale 'uk', got '%s'", symRU) | ||
} | ||
|
||
err = RegisterCurrency("BTC", RegisterCurrencyOptions{}) | ||
if err == nil { | ||
t.Error("Expected an error when re-registering code 'BTC', but got nil.") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's call this type just Definition. That gives you this DX: