0
\$\begingroup\$

This bubble sort code is very slow compared to the original.

import time
import random
def bubble_sort(a):
    #setting my variables
    yes=0
    d=0
    #making a switch
    while yes==0:
        if d>=last_thing-1:
            d=0
        if a[0+d]>a[1+d]:

            #switching the order
            a[0+d],a[1+d]=a[1+d],a[0+d]

        d+=1
        if correct_order(a):
            #turning off the switch
            yes=1

    return(a)
def correct_order(a):
    for i in range(len(a)):
        if i != a[i]:
            return(False)
    return(True)
if __name__ == '__main__':
    #setting your range
    last_thing=1000+1
    #making the random list
    a= list(range(0,last_thing))
    random.shuffle(a)

    #easier setting
    currentsortingmethod=bubble_sort(a)
    #prints your sorted list
    print("--------------------------------------------------------------------------------\n",str(currentsortingmethod).strip('[]').replace(" ", "").replace("'",""))
\$\endgroup\$
8
  • \$\begingroup\$ Hello and welcome to CodeReview. Your post needs some work. Please reformat it so that your code is all included in a code block. \$\endgroup\$ – Reinderien Oct 8 '19 at 22:55
  • \$\begingroup\$ @Renderien, I fixed it however it might not be working properly I'll check \$\endgroup\$ – bullseye Oct 8 '19 at 23:01
  • \$\begingroup\$ @RobbieAwesome This is not working code because it's incomplete last_thing and correct_order are not defined, fix it or this will count as off-topic and most probably will be closed. \$\endgroup\$ – bullseye Oct 8 '19 at 23:04
  • 3
    \$\begingroup\$ Then edit your post and provide a full working code \$\endgroup\$ – bullseye Oct 8 '19 at 23:09
  • 1
    \$\begingroup\$ @Reinderien It looks like maybe I jumped to early conclusions, it's currently working, I tested it. \$\endgroup\$ – bullseye Oct 9 '19 at 1:51
1
\$\begingroup\$

last_thing

You define last_thing and use it to initialise your array, which is fine, however you also use it within your actual bubble_sort function. This seems wrong, it would be better to use the length of the supplied array in order to determine when to stop. This will make your code more portable and able to handle different sized arrays.

When to check for correct_order

You only need to check if the array is sorted at the end of each pass. As it stands, you're checking if the array is sorted for each entry you're processing, even if you haven't done anything with the array. Which feels like you're turning an O(n^2) algorithm into an O(n^3) solution. Another approach you can use is to keep track of whether or not you've actually done a swap on a given pass, if you haven't then you know it's because the list is already sorted.

looping

This is probably very subjective, but I don't like this:

if d>=last_thing-1:
    d=0

To me, it makes the start of each pass through the list less obvious than other methods.

Generalisability

Algorithms are used to solve general problems. There are several instances where you've tied your solution to the exact problem (such as your use of last_thing) you're solving which means it can't solve the general problem. The way you've implemented correct_order is another good example of this. You iterate through the list and make sure that each item in the list has the same value as it's position in the list. This works for your exact problem, but makes your algorithm useless if you wanted to sort [8,2,4,6]. Consider writing unit tests so that you can actually exercise your code with different inputs to make sure they can be handled. This will help you to build code that is not so tightly coupled to the specific problem you're solving.

Odds and Bobs

  • You've imported time, but you're not using it. Redundant code adds unnecessary noise, consider removing the unused import.
  • Names matter. a and d aren't very descriptive, consider expanding the names to describe what it is they represent.
  • If you just need two values (0, 1), consider using a boolean instead (False/True).
  • Try to standardise on your spacing, sometimes you have spaces around operators sometimes you don't... it can make the code look messy.

Taking some of the above into account, a refactored version of your code might look something like this:

def bubble_sort(array):
    while not correct_order(array):
        for index in range(len(array) - 1):
            if array[0+index] > array[1+index]:
                array[0+index],array[1+index] = array[1+index],array[0+index]
    return(array)

def correct_order(array):    
    for i in range(len(array)-1):
        if array[i] > array[i+1]:
            return(False)
    return(True)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.