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\$
2
  • 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
    CommentedJun 27, 2019 at 5:29
  • \$\begingroup\$@RootTwo This looks like an answer, not a comment :)\$\endgroup\$CommentedJun 27, 2019 at 8:37

1 Answer 1

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\$
1
  • 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\$CommentedJun 27, 2019 at 13:07

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.