0
\$\begingroup\$

After implementing suggestions from an earlier, related question (Python: Selection sort), I have written this code. Could you help me to improve this code?

def get_input():
    input_str = input("Enter elements to be sorted: ")
    try:
        lst = list(map(int, input_str.split()))
    except:
        raise TypeError("Please enter a list of integers only, seperated by a space!!")
    return lst

def bubble_sort(thelist):
    if len(thelist) == 0:
        print("Empty list!!")

    elif len(thelist) == 1:
        print("Only one element!!")

    elif sorted(thelist) == thelist:
        print("List is already sorted!!")

    else:
        for i in range(len(thelist) - 1):

            for j in range(len(thelist) - i - 1):
                if thelist[j] > thelist[j+1]:
                    thelist[j], thelist[j+1] = thelist[j+1], thelist[j]

if __name__ == '__main__':
    input_list = get_input()
    bubble_sort(input_list)
    print(*input_list, sep = ", ")
\$\endgroup\$
  • 1
    \$\begingroup\$ Isn't sorted(thelist) == thelist a little bit unfair and (potentially) wasteful? \$\endgroup\$ – AlexV Jun 29 at 7:45
1
\$\begingroup\$

There is a big problem in your code. sorted sorts the input list. So you are sorting input list twice. For finding out that your list is sorted, you only need to check did you make any swap in the last iteration or not. If you don't, your list is sorted.

def get_input():
    input_str = input("Enter elements to be sorted: ")
    try:
        lst = list(map(int, input_str.split()))
    except:
        raise TypeError("Please enter a list of integers only, seperated by a space!!")
    return lst


def bubble_sort(the_list):
    if len(the_list) == 0:
        print("Empty list!!")

    elif len(the_list) == 1:
        print("Only one element!!")
    else:
        for i in range(len(the_list) - 1):
            swapped = False
            for j in range(len(the_list) - i - 1):
                if the_list[j] > the_list[j + 1]:
                    swapped = True
                    the_list[j], the_list[j + 1] = the_list[j + 1], the_list[j]
            if not swapped:
                break


if __name__ == '__main__':
    input_list = get_input()
    bubble_sort(input_list)
    print(input_list, sep=", ")

If you should print the List is already sorted!! message, you can simply create a copy of input list (using for example: input_list[::]) and compare that with the result of bubble_sort function in the last part of your code.

Also, many times changing the value of the function parameter and using that like an in-out parameter is not a very good idea in the python. Because that could create many problems and misunderstandings in the future, especially when you are working with other people.

So It is better to think about using this method or returning a new sorted list from function.

But if you want to stick to the current method, don't forget to explain that in the documentation of your code clearly.

Also, for making your code more readable, maybe it is a good idea to split your bubble_sort function to two separate functions. One checks conditions and calls the second one that really sorts the list. But it is not a necessary thing and, depends on your team and project code style and decisions, maybe make things wors. That is a thing that you can consider in your mind and think about it.

\$\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.