CODEWITHSUNDEEP
pythonpython-3.x
Muckity Muck
Muckity Muck

Reputation: 23

If statement is not branching correctly

I am a novice at python. The program was to pass any list into a function and print a string with the words separated with a ',' with the last word separated by a ', and'. But the If statement does not branch for the last word. I don't know if putting it in a for loop is breaking it.

spam = ['apples', 'oranges', 'pears', 'strawberries', 'watermelon']

def comma(x):
    newSpam = x[0]
    del x[0]
    for i in range(len(x)):
        print(type(len(x)))
        if i == (len(x)):
            newSpam = newSpam + ', and ' + [x(i)]
        else:            
            newSpam = newSpam + ', '+ x[i]
    print(newSpam)
comma(spam)

The output I get is:

apples, oranges, pears, strawberries, watermelon

Upvotes: 2

Views: 51

Answers (4)

ShadowRanger
ShadowRanger

Reputation: 155458

While the other answers cover fixing your code as written, your code as written has a couple significant flaws:

  1. It's building strings by repeated concatenation, which is O(n**2) work (it's a form of Schlemiel_the_Painter's_algorithm)
  2. You're mutating the argument (deleting the first value), which can be quite surprising for an output-oriented function

I'd suggest a refactoring to avoid both problems (and simplify the code):

def comma(x):
    *rest, last = x  # Unpacks to a new list of all the values but one, with last separated
    if rest:
        # For more than one item, join rest with commas, and explicitly
        # add separator between rest and last
        print(', '.join(rest), last, sep=', and ')
    else:
        # For one item, just print it
        print(last)

As a side-benefit of this transformation, the function no longer requires a mutable sequence (e.g. list); it works equally well with (non-mutable) tuple, or (non-sequence) set (though the ordering would be arbitrary), or a generator without known length (because *rest, last = x is converting the input to a list and a single value, regardless of the input type).

Upvotes: 0

Land Owner
Land Owner

Reputation: 182

length does not indicate the last item of a list.You need to use length-1 to indicate the last item.

def comma(a_list):
   x=len(a_list)
   y=""
   for i in range (x):
     if i==(x-1):
        y=y+"and "
        y=y+spam[i]
        break
     y=y+spam[i]
     y=y+", "

    print (y)

Upvotes: 0

s.py
s.py

Reputation: 197

As you've demonstrated in the newSpam = x[0] line, Python is zero-indexed, which means that the first item of the list is index 0 and the last item of the list is one less than the length of the list. Thus, to check if you're at the last item, you need to check if i == (len(x) - 1) rather than if i == len(x).

Upvotes: 2

Harald Nordgren
Harald Nordgren

Reputation: 12399

Replace if i == (len(x)): with

if i == (len(x) - 1):

Upvotes: 1

Related Questions