1

I need to add one to all even and all odd numbers in the list but whenever i run it it just prints the same random list twice. Most recent output:

[3, 58, 40, 50, 21, 33, 25, 20, 37, 27, 21, 37, 11, 30, 75, 16, 95, 18, 37, 55]
[3, 58, 40, 50, 21, 33, 25, 20, 37, 27, 21, 37, 11, 30, 75, 16, 95, 18, 37, 55]

This is my code:

import random
randomlist = []
for i in range(20):
randomlist.append(random.randrange(1,101))    
print(randomlist)

def make_odd(randomlist):

    for i in range(len(randomlist)):
        if i % 1 != 0:
            randomlist.append(i+1)
        
        return randomlist

print(randomlist)
Patrick Artner
  • 50,409
  • 9
  • 43
  • 69
Pobi
  • 19
  • 4
  • All numbers (positive integers to be accurate) are divisible by 1, so this `i % 1 != 0` is always false – Dani Mesejo Nov 20 '20 at 22:16
  • and you add a new number to the list thats 1 bigger so the loop might not terminate if you do it for odd and even numbers. You probably need `for index,value in enumerate(randomlist):` and `if value%2 == 1: randomlist[index]+=1` – Patrick Artner Nov 20 '20 at 22:18
  • I would recommend reading https://ericlippert.com/2014/03/05/how-to-debug-small-programs/. – AMC Nov 20 '20 at 22:19
  • What exactly is `make_odd` supposed to do? Does it add 1 to all odd numbers, add 1 to all even numbers, or what? – M-Chen-3 Nov 20 '20 at 22:30

3 Answers3

2

You add a new number to the list thats 1 bigger so the loop might not terminate if you do it for odd and even numbers. The modulo operation is always False. You never call your function. There are more problems, see M-Chen-3's answer.

As suggested in my comment above his code uses for index,value in enumerate(randomlist): and if value%2 == 1: randomlist[index]+=1 to change the data.

You can streamline your code with more advanced python a lot:

import random

def make_odd(data):
    # oddify values if even else keep the odd one around
    return [n+1 if n%2 == 0 else n for n in data]

randomlist = random.choices(range(1,101),k=20)

print(randomlist)
print(make_odd(randomlist))

Output:

[41, 3, 23, 83, 37, 13, 82, 25, 66, 10, 97, 98, 75, 76, 71, 30, 53, 7, 22, 13] 

[41, 3, 23, 83, 37, 13, 83, 25, 67, 11, 97, 99, 75, 77, 71, 31, 53, 7, 23, 13]

if you use a different random method (choices) that gives you a list of random values from a range. Using a list comprehension to modify your data to odd values is also shorter and neater. Read more about list comprehension with conditions here: if/else in a list comprehension

Patrick Artner
  • 50,409
  • 9
  • 43
  • 69
1

There are 5 problems with your code.

  1. The indentation and spacing is nonexistant. This makes your code extremely hard to understand and will cause problems with how your program functions.

  2. When you do for i in range(len(randomlist)), i is the index of an item, not the item itself. Therefore, to access the item you must do randomlist[i], not i.

  3. As Dani Mesejo mentioned, i % 1 != 0 will always return False. I assume by the name of make_odd that you want to make every number odd, so do i % 2 == 0 as this will check if the number is even.

  4. Instead of appending a new element to randomlist, you should change the element itself by doing randomlist[i] = #something.

  5. At the very end, you never even call the make_odd function! print(randomlist) What you SHOULD do is print(make_odd(randomlist)). This calls the function then prints out the result.

Hopefully my solution has helped you understand the process better.

import random

randomlist = []

for i in range(20):
    randomlist.append(random.randrange(1,101))    
print(randomlist)

def make_odd(randomlist):
    for i in range(len(randomlist)):
        # Operating on randomlist item itself
        # instead of i, the index
        if randomlist[i] % 2 == 0:
            # Updating the item instead of adding a new one
            randomlist[i] += 1

    return randomlist

# Calling the function with make_odd
print(make_odd(randomlist))
M-Chen-3
  • 2,036
  • 5
  • 13
  • 34
  • 1
    ah sorry about the indentation i dont post alot on here... it wouldnt let my code be in the "code" bit without me meddling with it also the 1 was a typo and i should have looked over it, turns out in a previous iteration i was so close but thanks to you and the others i understand the issue so thanks. – Pobi Nov 21 '20 at 00:37
1

I agree with the issues pointed out by M-Chen-3 but the solution by Patrick Artner. Simply because these 2 things:

  • I would avoid modifying a list I'm actively looping
  • Why are you iterating over the positions instead of the actual objects?

unstreamlined would look like a combination of these 2 answers

def make_odd(randomlist):
    odd_list = []

    for i in randomlist:
        if i % 2 == 0:
            i += 1
        odd_list.append(i)

    return odd_list


my_list = [3, 68, 19, 3, 51, 79, 7, 43, 6, 45, 20, 19, 78, 90, 76, 15, 56, 93, 83, 92]
print(my_list)
print(make_odd(my_list))
Boomer
  • 464
  • 3
  • 10
  • 1
    Modifying a list while iterating is save UNLESS you lenghten or shorten it. Modifying values might have sideeffects if you need the "starting values" of the list to compute new value at the current position - then you could iterate a copy instead. Never lenghten or shorten a list while iterating. And a list comp is a fancy 1-liner for what you do in 5 lines :) – Patrick Artner Nov 20 '20 at 22:39
  • 1
    Yes, I agree. That is why I upvoted your answer. I just wanted to include what may be easier to read for beginners. I know also you CAN change lists while iterating, but disagree that you ever should just as good practice. – Boomer Nov 20 '20 at 22:41