0
\$\begingroup\$

Sorted List value using algorithm selection sort. Can you please review and let me know where I can improve.

months = {"January": 1, "February": 2, "March": 3, "April": 4, "May": 5, "June": 6, "July": 7, "August": 8, "September": 9, "October": 10, "November": 11, "December": 12}

L = ["March", "January", "December"]

#creating list L1
L1 = [months[L[i]] for i in range(len(L))]
print(L1)

#sorting list using algorithm selection sort
for i in range(1,len(L1)):
    j = 0
    if L1[j] > L1[i] :
        L1[j], L1[i] = L1[i] ,L1[j]
print(L1)

#Replacing values with key
sorted_list = [ k   for k,v  in months.items() if v in L1]
print(sorted_list)
\$\endgroup\$
4
\$\begingroup\$

Put your code in functions. Maybe this is pedantic, but it's not really an algorithm if you cannot reuse it. If the logic is floating around with various setup and printing code, it's not reusable in any convenient sense.

Your demo code isn't doing what you think it's doing. Comment out the sorting code, and the printed result is the same: the month names in proper order. Why? Because sorted_list is being driven by the ordering of the months dict, not the we-hope-its-sorted L1. As an experiment, move January to the end of that dict, and watch what happens.

# The order of sorted_list is not affected by whether L1 is sorted.
sorted_list = [k for k, v in months.items() if v in L1]

Even if it were fixed, your demo code is not well-suited to the problem at hand -- checking whether your sort algorithm is correct. Instead, your demo code is good for addressing a different question: how to sort dict keys based on dict values. Here's one way:

# Sorting dict keys based on dict values -- a different question.
sorted_names = sorted(months, key = lambda name: months[name])

A better way to test your sorting algorithm is to shuffle the values, sort them with your algorithm, and check your sorted list against what the Python sort() function gives us. It's easy to write and a good way to check many more cases than anything you could manually type. Here's one illustration of how you might check it. You could easily wrap a loop around this kind of testing to include a wider range of list sizes (including edge cases like vals = [] or vals = [1]).

from random import shuffle

def main():
    # Some data that needs sorting.
    vals = list(range(1, 13))
    shuffle(vals)

    # Sort both ways and compare.
    expected = sorted(vals)
    selection_sort(vals)
    if vals != expected:
        print('GOT', vals)
        print('EXP', expected)

def selection_sort(L1):
    # L1 is a non-standard Python variable name. In a general sorting
    # function, where you have no idea what the values represent,
    # an idiomatic name would be xs (one x, many xs).
    for i in range(1,len(L1)):
        j = 0
        if L1[j] > L1[i] :
            L1[j], L1[i] = L1[i] ,L1[j]

if __name__ == '__main__':
    main()

Your sorting algorithm isn't working. Here's output from one run:

GOT [1, 7, 8, 4, 12, 10, 2, 11, 6, 5, 9, 3]
EXP [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]

If you add these suggestions to your code, and do some debugging work, I'm sure you'll figure out a correct implementation. For example, you are not finding the index of the minimum value in the rest of the list -- a key concept in Wikpedia's description of selection sort. Come back for another code review after you have another draft.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for explanation. I could figure out where I was going wrong. I have posted code below. Please review. \$\endgroup\$ – ch2019 Jun 8 at 20:34
  • 2
    \$\begingroup\$ @ch2019 You are very welcome and your new implementation is much closer. But take a look at that Wikipedia page -- especially the animation and the corresponding pseudo-code. Each pass over the list checks only the rest of the list (the growing yellow portion in the animation). Your new code is checking the entire list with each pass, so you're doing too much work. Also, I don't know all of the rules around here, but I suspect the policy is to post subsequent drafts for review as brand new questions rather than as alternative answers (that's just a guess on my part). \$\endgroup\$ – FMc Jun 8 at 21:36
  • \$\begingroup\$ @ch2019 Also, your new code is still missing the essence of selection sort: finding the minimum value in the rest of the list. \$\endgroup\$ – FMc Jun 8 at 22:00

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.