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

Fixed median algorithm bug #484

Merged
merged 2 commits into from
Oct 6, 2017
Merged

Conversation

j-haj
Copy link
Contributor

@j-haj j-haj commented Oct 2, 2017

Fixes #436

Modified the algorithm slightly to first find the median value, then find the index corresponding to that value. The basic idea is to sum the values, then remove the max and min values, leaving the median as the result.

By submitting this pull request I confirm I've read and complied with the below declarations.

  • I have read the Contribution guidelines and I am confident that my PR reflects them.
  • I have followed the coding guidelines for this project.
  • My code follows the skeleton code structure.
  • This pull request has a descriptive title. For example, Added {Algorithm/DS name} [{Language}], not Update README.md or Added new code.
  • This pull request will be closed if I fail to update it even once in a continuous time span of 7 days.

@singhpratyush
Copy link
Member

Hi! Thanks for showing interest in contributing to this repository.

Please make sure that you have ticked the points mentioned in PR description correctly.

Thanks again!

return i
elif aj == med_val:
return j
return k
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not comply to PEP8.

Origin: PEP8Bear, Section: pep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp4__xq2to/quick_sort/quick_sort.py
+++ b/tmp/tmp4__xq2to/quick_sort/quick_sort.py
@@ -14,6 +14,7 @@
     elif aj == med_val:
         return j
     return k
+
 
 def partition(array, l, r):
     """

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes - my apologies. My pep8 vim plugin did not catch this for some reason.... updating the PR now!

@j-haj j-haj force-pushed the issue436-fix-quicksort branch from 48f051e to b280c26 Compare October 2, 2017 23:11
@iiitv iiitv deleted a comment Oct 2, 2017
@j-haj
Copy link
Contributor Author

j-haj commented Oct 2, 2017

@singhpratyush I added an extra newline and updated the commit

Copy link
Member

@Monal5031 Monal5031 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
ping @singhpratyush

@Monal5031 Monal5031 changed the title Fixes issue 436 - median algorithm bug Fixed median algorithm bug Oct 5, 2017
@singhpratyush singhpratyush merged commit b1b9e13 into iiitv:master Oct 6, 2017
@iiitv iiitv deleted a comment Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Condition is not fully correct in quicksort [Python]
3 participants