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

Get Currency by Numeric Code #104

Merged
merged 2 commits into from
Mar 31, 2022
Merged

Get Currency by Numeric Code #104

merged 2 commits into from
Mar 31, 2022

Conversation

Rhymond
Copy link
Owner

@Rhymond Rhymond commented Mar 31, 2022

Code changes allow to get the currency by numeric code or a code used to index currencies map. The PR fixes issues with the PR #96

@Rhymond Rhymond self-assigned this Mar 31, 2022
@Rhymond Rhymond merged commit e8fc0e2 into master Mar 31, 2022
@Rhymond Rhymond deleted the currency-by-code branch March 31, 2022 15:59
@psrajat
Copy link

psrajat commented Mar 31, 2022

@Rhymond Do you feel the CurrencyByNumericCode will be under performant compared to the provided fix by @Stocco?
Can't we update the currencies map when we are adding a new currency?

@Rhymond
Copy link
Owner Author

Rhymond commented Mar 31, 2022

@psrajat I don't think so, we're talking about the map of 100 items, trust me Go is fast enough to read from memory and make it barely noticeable. Of course it highly depend on your use case. If each nanosecond (billionth of a second) counts, you could use Add() method to add currencies to the list by NumericCode and get it by the code then.

@psrajat
Copy link

psrajat commented Apr 4, 2022

@Rhymond Sorry for getting back late.

If each nanosecond (billionth of a second) counts

TBF we don't even have this use case as of now.
But I feel it should be pointed out that iteration performance of a map is quite poor because of the underlying data structure.

I actually ran some benchmarks of CurrencyByCode() and CurrencyByNumericCode() methods in my local machine and I observe almost a 100x difference.

var result *Currency

func BenchmarkCurrencies_CurrencyByNumericCode(b *testing.B) {
	var currency *Currency
	for n := 0; n < b.N; n++ {
		currency = currencies.CurrencyByNumericCode("932")
	}
	result = currency
}

func BenchmarkCurrencies_CurrencyByCode(b *testing.B) {
	var currency *Currency
	for n := 0; n < b.N; n++ {
		currency = currencies.CurrencyByCode("ZWD")
	}
	result = currency
}
BenchmarkCurrencies_CurrencyByNumericCode
BenchmarkCurrencies_CurrencyByNumericCode-8   	 1218165	        942.8 ns/op
BenchmarkCurrencies_CurrencyByCode
BenchmarkCurrencies_CurrencyByCode-8          	100000000	        10.59 ns/op

A high throughput service which relies on CurrencyByNumericCode() can add this bit of optimization.
And I feel it should be the part of the library rather than manually adding the same currencies using Add().

@psrajat
Copy link

psrajat commented Apr 22, 2022

@Rhymond Any thoughts on this?

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.

None yet

2 participants