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

P.times() carry problem #125

Closed
jsyt opened this issue Aug 2, 2019 · 7 comments
Closed

P.times() carry problem #125

jsyt opened this issue Aug 2, 2019 · 7 comments

Comments

@jsyt
Copy link
Contributor

jsyt commented Aug 2, 2019

big.js line 848

c[j] = (c[j] + b) % 10;

If there is a carry here, then you have not processed the carry.

If there is no carry I think the following is better.

c[j] =  b;

I don't think there will be a carry in here.

So if I replace c[j] = (c[j] + b) % 10; with c[j] = b;, will there be any problem here?

Thank you!

@MikeMcl
Copy link
Owner

MikeMcl commented Aug 2, 2019

I don't think there will be a carry in here.

b is the carry, so I do not know what you mean here, or in the other sentences in which you have used the word "carry".

Without meticulously studying this time-tested algorithm I wrote years ago, I cannot be sure if it is safe to replace c[j] = (c[j] + b) % 10; with just c[j] = b;. There may be edge cases not included in the tests that mean it cannot be.

Will c[j] will always be zero at that point and will b always be equal to b % 10?

@jsyt
Copy link
Contributor Author

jsyt commented Aug 3, 2019

b is the carry, so I do not know what you mean here, or in the other sentences in which you have used the word "carry".

I mean that if c[j] != 0, then(c[j] + b) may get a carry, and you have not processed the carry.

Will c[j] will always be zero at that point and will b always be equal to b % 10?

Yes, I think that c[j] will always be zero at that point and b always be equal to b % 10.

Thank you!

@MikeMcl
Copy link
Owner

MikeMcl commented Aug 3, 2019

You just "think" or are you absolutely 100% sure? Unfortunately, I am not able to study this closely at the moment.

@jsyt
Copy link
Contributor Author

jsyt commented Aug 4, 2019

You just "think" or are you absolutely 100% sure? Unfortunately, I am not able to study this closely at the moment.

I am absolutely 100% sure that c[j] will always be zero at that point and b always be equal to b % 10.
And I replace c[j] = (c[j] + b) % 10; with c[j] = b;, it passed all the test cases.

@MikeMcl
Copy link
Owner

MikeMcl commented Aug 4, 2019

Okay, I will accept a pull request with this change, if you so wish.

I sure hope you know what you are doing.

jsyt added a commit to jsyt/big.js that referenced this issue Aug 5, 2019
jsyt added a commit to jsyt/big.js that referenced this issue Aug 5, 2019
MikeMcl added a commit that referenced this issue Aug 5, 2019
@MikeMcl MikeMcl closed this as completed Aug 5, 2019
@dkent600
Copy link

Seems like would be good to include a test case that goes from red to green by this.

@MikeMcl
Copy link
Owner

MikeMcl commented Aug 16, 2019

@dkent600

This wasn't a bug, just a minor optimisation.

BTW I was speaking tongue-in-cheek when I wrote "I sure hope you know what you are doing" above.

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

No branches or pull requests

3 participants