0

The algorithm works as follow:

  1. I find the minimum of a given list.
  2. Then pop it and append it to another list.
  3. Repeat step 1 and 2 until there is one element, in which case i simply append it to the other list and end the program.

Issue

The last element is always some random number which should be sorted long ago.

Source Code

lst=[randrange(1, 100) for i in range(100)]
lst2=[]
while True:
    if len(lst) > 1:
        min = 0
        for i in range(len(lst) -1):
                if min == 0:
                    min = lst[i]
                else:
                    if lst[i] < min:
                         min = lst[i]
        for j in range(len(lst) -1):
            if lst[j] == min:
                lst2.append(lst[j])
                lst.pop(j)
                break
    else:
        lst2.append(lst[0])
        break
lst = lst2
print(lst)
Improve this question
9
  • 3
    range's end argument is exclusive. So it should just be for i in range(len(lst)): Commented Dec 28, 2020 at 13:45
  • 4
    By the way, list.pop returns the popped element, so it can just be: lst2.append(lst.pop(j)) Commented Dec 28, 2020 at 13:46
  • You can also use index to get the index without explicitly iterating and min to get the minimum value in the same manner - it'll give you far more readable code. Commented Dec 28, 2020 at 13:51
  • Yeah the range was the problem, thanks a lot! Thanks for the other tip too Commented Dec 28, 2020 at 13:51
  • 1
    @MatsLindh they can also just lst.sort()... Commented Dec 28, 2020 at 13:52

2 Answers 2

Reset to default
3

Your code has just one minor flaw. As @Tomerikoo pointed out already, there is just a little mistake at the iterators. The correct code would look like this:

lst=[randrange(1, 100) for i in range(100)]
lst2=[]
while True:
    if len(lst) > 1:
        min = 0
        for i in range(len(lst)):
                if min == 0:
                    min = lst[i]
                else:
                    if lst[i] < min:
                         min = lst[i]
        for j in range(len(lst)):
            if lst[j] == min:
                lst2.append(lst[j])
                lst.pop(j)
                break
    else:
        lst2.append(lst[0])
        break
lst = lst2
print(lst)

There is a bit more elegant implementation, that iterates over the list items instead of just the indices.

lst=[randrange(1, 100) for i in range(100)]
lst2=[]
while True:
    if len(lst) > 1:
        min = 0
        for item in lst:
                if min == 0:
                    min = item
                else:
                    if item < min:
                         min = item
        for idx, item in enumerate(lst):
            if item == min:
                lst2.append(item)
                lst.pop(idx)
                break
    else:
        lst2.append(lst[0])
        break
lst = lst2
print(lst)

In the one case where you actually need an index an enumerate is your tool of choice. This improvement makes your code easier to read in general and utilizes one of the features of Python instead of, for example, C.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Worth noting that the if/else can also be removed by changing the loop condition to be while len(lst) > 0
1

  1. Change for i in range(len(lst) -1): to for i in range(len(lst)):

  2. You can improve the Algorithm by finding the index directly without loping twice as so:

from random import randrange


lst = [randrange(1, 100) for i in range(100)]
lst2 = []
while True:
    if len(lst) > 1:
        min = 0
        for i in range(len(lst)):  # FIND min value
                if not min:
                    min = lst[i]
                else:
                    if lst[i] < min:
                         min = lst[i]
        get_index = lst.index(min)  # Get index of Value
        min_value = lst.pop(get_index) # Pop min value
        lst2.append(min_value)  # Append min Value
    else:
        lst2.append(lst[0])
        break
lst = lst2
print(lst)

EDIT

@MatsLindh & @Tomerikoo Pointed out that index function internally runs a loop (so basically is the same) but just more readable

Hence the following code would be much cleaner and with a better performance:

from random import randrange


lst = [randrange(1, 100) for i in range(100)]
lst2 = []
while True:
    if len(lst) > 1:
        min = 0
        idx = 0
        for i in range(len(lst)):  # FIND min value
                if not min:
                    min = lst[i]
                    idx = i
                else:
                    if lst[i] < min:
                        min = lst[i]
                        idx = i  #Store Index of min_value
        min_value = lst.pop(idx) # Pop min value
        lst2.append(min_value)  # Append min Value
    else:
        lst2.append(lst[0])
        break
lst = lst2
print(lst)

Guides

Improve this answer

6 Comments

index will do the same loop internally; instead you can keep the value of i when you find a new min and pop that one.
It that so? so basically is just Syntactic sugar?
ok makes sense, by the way I meant that using the index function won't make the time complexity any better? Because Than1 initially was running 2 for loops so I removed the second for loop and added the index function instead, but basically it does the same thing right?
Yes, under the hood it's still the same: looping until the first occurrence of the element and returning its index
Of course, it is still more readable to use index than a loop... So it is still preferable. But, again, in this case you can just save the matching i when you found the minimum and avoid the index loop altogether
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.