go-money icon indicating copy to clipboard operation
go-money copied to clipboard

1.18 Generics

Open Rhymond opened this issue 2 years ago • 4 comments

Add 1.18 generics to go-money to allow working with different numeric types of Money (e.g. int, int64)

  • JSON marshal/unmarshal not yet working properly
  • Write tests to make sure all numeric types defined in Numeric interface are tested

Rhymond avatar Mar 31 '22 09:03 Rhymond

Hold up, you are not restricting type anywhere. Users may pass say float64, and loose the main benefit of this library — decimal integer arithmetics.

If adding generics, please restrict only to integer types.

Also, lots of enterprise code is not migrated to 1.18 yet. By enforcing 1.18 (breaking change?) lots of people can not use this library anymore. Please make new version or advise how this library still can be used with 1.13.

Generics are nice, but afraid would be dangerous for this project.

nikolaydubina avatar Apr 08 '22 08:04 nikolaydubina

Hold up, you are not restricting type anywhere. Users may pass say float64, and loose the main benefit of this library — decimal integer arithmetics.

If adding generics, please restrict only to integer types.

Also, lots of enterprise code is not migrated to 1.18 yet. By enforcing 1.18 (breaking change?) lots of people can not use this library anymore. Please make new version or advise how this library still can be used with 1.13.

Generics are nice, but afraid would be dangerous for this project.

Hi @nikolaydubina There is a type constrain added : https://github.com/Rhymond/go-money/pull/103/files#diff-d48dd271ebf1115ce9c4ee56a566b1d006eee1bc2e3211630b302c717464f964R70

type Numeric interface {
	~int | ~int8 | ~int16 | ~int32 | ~int64
}

It only allows different types of int, still working on implementing uint and possibly bigint (going to create a new PR for that)

At the start I was thinking to roll up a new major version, but it looks like that existing tests are all passing fine, which means there are no breaking changes. There are still work needed to be done in regards to custom json marshalling.

Rhymond avatar Apr 08 '22 08:04 Rhymond

@nikolaydubina What's the benefit of making money generic this way? The only one I can see is to save a little of space. The performance will not be better in modern processors, and memory is relatively cheap. It also introduce complexity in its use.

Additionally, is a money based on "int8" useful?

totemcaf avatar Jul 31 '22 16:07 totemcaf

The benefit is that it is not possible to pass float types. One of selling points for go-money is numerical precision, which is gone once floats are used internally.

the performance will not be better in modern processors, and memory is relatively cheap. It also introduce complexity in its use.

yes

is a money based on "int8" useful?

not really, unless you running on some special hardware. but then likely you would be using C instead

But then again, please check with project owner to align on this! :)

nikolaydubina avatar Aug 01 '22 02:08 nikolaydubina