Skip to content

added counting sort #133

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

Closed
wants to merge 1 commit into from
Closed

added counting sort #133

wants to merge 1 commit into from

Conversation

sgbasaraner
Copy link

@sgbasaraner sgbasaraner commented Sep 30, 2017

hacktoberfest: I copied the comments from bubble_sort and left them as is (changed bubble_sort to counting_sort), and my counting sort implementation works with negative numbers too. if the comments will be a problem, notify me and I'd like to make the necessary changes.

@andrealmeid andrealmeid mentioned this pull request Oct 14, 2017
@andrealmeid
Copy link
Contributor

andrealmeid commented Oct 14, 2017

Hello @sgbasaraner,
I read your code and I found some issues that you might be interest to notice. Two main features of counting sort is linear time complexity and stability. But, when you do this loop in lines 32-33

for i in range(counting_arr_length):
    for j in range(length):

you are, in fact, running j * i times. This can easily achieve O() time complexity, when i >= j.

When you create the array, you do

counting_arr_length = maximum + 1 + abs(minimum)

But we don't need all this positions, just maximum + 1 - minimum are enough to store all the values.

Finally, the stability. Stability in counting sort is an important feature because it makes counting sort useful to be used as a radix sort subroutine. The way you do it, you lose the previous information about the original order of the elements.

I suggest checking out some Wikipedia pages about sorting algorithms and Introduction to Algorithms by Cormen, Leiserson, Rivest and Stein to find more about stability and time complexity. I made my own version, check it out! (#165) Contact me if you have doubts or need more information :)

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.

3 participants