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

Reconcile DecCoin/s with Coin/s #3311

Closed
4 tasks done
alexanderbez opened this issue Jan 16, 2019 · 4 comments · Fixed by #3607
Closed
4 tasks done

Reconcile DecCoin/s with Coin/s #3311

alexanderbez opened this issue Jan 16, 2019 · 4 comments · Fixed by #3607
Assignees
Milestone

Comments

@alexanderbez
Copy link
Contributor

alexanderbez commented Jan 16, 2019

Summary

Reconcile the DecCoin and DecCoins APIs with the Coin and Coins APIs for consistency.

e.g.

  • Not allowing negative amounts
  • Not returning zero amounts in plus
  • Consistent constructors and naming conventions
  • Improve godocs & test coverage
  • etc...

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@rigelrozanski
Copy link
Contributor

What did you have in mind? like testing through an interface? golang is kinda inhibiting for actually combining these two types unfortunately

@alexanderbez
Copy link
Contributor Author

Yeah I wish we could keep it DRY as possible, but I mean just making sure the APIs are the same (e.g. plus doesn't make sure zero coins are removed).

@alessio
Copy link
Contributor

alessio commented Jan 22, 2019

How about defining these common interfaces?

type Coin interface {
	SameDenomAs(Coin) bool
	Zero() bool
	GTE(Coin) bool
	LT(Coin) bool
	Equal(Coin) bool
	Plus(Coin) Coin
	Minus(Coin) Coin
	Positive() bool
	Negative() bool
	String() string
}

type CoinSet interface {
	Valid() bool
	Plus(CoinSet) Coin
	Minus(CoinSet) Coin
	SafeMinus(CoinSet) Coin
	AllGT(CoinSet) bool
	AllGTE(CoinSet) bool
	AllLTE(CoinSet) bool
	Zero() bool
	Equal(CoinSet) bool
	Empty() bool
	AllPositive() bool
	AllNotNegative() bool
        AnyNegative() bool
	Sort() CoinSet
	String() string
}

Note that I have removed the Is prefix from all types - IMHO it does not add any value to the methods names.

@alessio
Copy link
Contributor

alessio commented Jan 22, 2019

We might need an Amount interface to allow Int/Dec comparisons

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants