1
\$\begingroup\$

After implementing suggestions from an earlier, related question (Python: Insertion 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: ")
    lst = list(map(int, input_str.split()))
    return lst

def selection_sort(thelist):
    for i in range(len(thelist)-1):
        min_idx = i

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

        thelist[i], thelist[min_idx] = thelist[min_idx], thelist[i]


if __name__ == '__main__':
    input_list = get_input()
    selection_sort(input_list)
    print(*input_list, sep = ", ")
\$\endgroup\$
  • 1
    \$\begingroup\$ It looks like a correct implementation of the sorting algorithm, so good job. The input prompt could be more explicit that it is expecting a sequence of integers separated by spaces. get_input() could handle errors in the input (e.g. if the user enters "a, b ,c"). You could add doc strings and you could add some unit tests for selection_sort() (e.g., sort and empty list, a one element list, an already sorted list, etc.) \$\endgroup\$ – RootTwo Jun 27 at 5:29
  • \$\begingroup\$ @RootTwo This looks like an answer, not a comment :) \$\endgroup\$ – Mathias Ettinger Jun 27 at 8:37
3
\$\begingroup\$

Regarding the code itself I think functions should usually return an output and then this should be printed. It is also worth introducing some way of alerting the user if there input causes an error. I entered 5, -3, 0 and this raised an error because your code splits on spaces not commas. Additionally, you are mapping the list to int so entering a character by mistake breaks the code

Hence saying something like

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, separated by a space")
    return lst

def selection_sort(thelist):
    for i in range(len(thelist)-1):
        min_idx = i

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

        thelist[i], thelist[min_idx] = thelist[min_idx], thelist[i]
    return thelist

if __name__ == '__main__':
    input_list = get_input()
    output = selection_sort(input_list)
    print(*output, sep = ", ")
\$\endgroup\$
  • 2
    \$\begingroup\$ I don't agree on there needing to be an output. Python convention is that methods that change a list inplace (list.sort, list.extend,...) do not return a value. I think this is to prevent the assumption that you need the assignment, and the original input is untouched. Here you mutate the input, and return the value. I would choose either of the 2. Your remark on get_input are correct. I would however surround this with a while True:, print the error msg instead of raising the Exception. and give the user a new chance to give input in the correct format \$\endgroup\$ – Maarten Fabré Jun 27 at 13:07

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.