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

Migrate extended Euclidean algorithm #202

Merged
merged 3 commits into from
Sep 14, 2018

Conversation

alxmjo
Copy link
Collaborator

@alxmjo alxmjo commented Sep 13, 2018

Migrate extended Euclidean algorithm to header file (as described in #163).

@alxmjo alxmjo self-assigned this Sep 13, 2018
@alxmjo alxmjo added this to To do in Reorganise project via automation Sep 13, 2018
@faheel
Copy link
Member

faheel commented Sep 13, 2018

@alxmjo Please rebase this with the latest changes from reorganise.

Refactor greatest common denominator algorithm to use a header file. Add test
cases as .cpp file to test. Also fix an error in greatest common denominator
function that prevented the accurate calculation of negative arguments. Update
README.
Removes old .cpp file for the extended Euclidean algorithm and adds unit
 tests. Also updates README.
@alxmjo
Copy link
Collaborator Author

alxmjo commented Sep 14, 2018

@faheel Not sure why this ended up bringing in the older version of the greatest common divisor files. This is what it looks like for me locally: alxmjo@11cce22

@alxmjo
Copy link
Collaborator Author

alxmjo commented Sep 14, 2018

I think it's OK now. Apologies for the mixup.

@faheel faheel merged commit 3f4bb45 into ProAlgos:reorganise Sep 14, 2018
Reorganise project automation moved this from To do to Done Sep 14, 2018
@alxmjo alxmjo deleted the reorganise-extendedeuclidean branch September 14, 2018 13:35
@ankur54
Copy link

ankur54 commented Oct 4, 2018

Can anyone please specify the work/issue at hand? I would like to start working on it.

@alxmjo
Copy link
Collaborator Author

alxmjo commented Oct 4, 2018

@ankur54 This is closed and no work needs to be done. I just shared it as a reference for how to go about refactoring the code into header files.

@ankur54
Copy link

ankur54 commented Oct 4, 2018

ok! So what about Lowest common ancestor in Binary Search tree? Is it also completed?

@alxmjo
Copy link
Collaborator Author

alxmjo commented Oct 4, 2018

If you see it in the reorganise branch under /include, then yes. If not, no. But I do know that @faheel had planned some reorganisation with regards to graphs, so he's the authority on that.

@ankur54
Copy link

ankur54 commented Oct 4, 2018

I'm sorry, but I can't find any such file. So I assume its still not resolved. In that case, I would like to work on it.

@alxmjo
Copy link
Collaborator Author

alxmjo commented Oct 4, 2018

No problem, I just didn't take the time to check myself. 🙂Please do. But start an issue for it first so that others know that it's being implemented.

@alxmjo alxmjo mentioned this pull request Oct 4, 2018
3 tasks
@alxmjo alxmjo mentioned this pull request Oct 19, 2018
3 tasks
@alxmjo alxmjo mentioned this pull request Oct 30, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants