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

Sample fix for issue 404 #613

Closed
wants to merge 1 commit into from
Closed

Sample fix for issue 404 #613

wants to merge 1 commit into from

Conversation

ericlagergren
Copy link

I noticed somebody else ran into this snag, so I decided it might be a good idea to re-look at the problem.

Unfortunately, there's not much I can do on my end. There are trade offs between what other packages do:

type T { x *big.Int }
func (x T) Add(y T) T { ... }

and what my package does:

type T { x big.Int }
func (z *T) Add(x, y *T) *T { ... }

One of these is the inability to unmarshal into a nil pointer.

So, this is just an example of one way to fix the problem. I'm sure there are others. But if this is continuing to affect folks like in ericlagergren/decimal#141, it might be good to re-open the conversation.

@aarondl
Copy link
Member

aarondl commented Oct 9, 2019

I appreciate this. Not sure why I didn't think of this as a basic workaround. Thanks. I've taken the code and rewritten the test (panicing tests are fine imho) and also applied the same logic to Decimal not just NullDecimal. I'll credit you with the fix in the changelog however :)

@aarondl aarondl closed this Oct 9, 2019
@ericlagergren
Copy link
Author

Happy to help!

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.

2 participants