Comma Code (Python)

Question

This is a practice from "Automate the boring stuff with Python".

My code works, but after checking out some other solutions on the Internet my solution looks more basic, and I can't tell if that's a good thing or not and if I got the right lessons from this chapter.

This is the code:

def items(someList):
    stringEnd = str(someList[-1])
    del someList[-1]
    str1 = ', '.join(someList)
    new_string = ''
    new_string = str1 + ' and' + ' ' + stringEnd
    return(new_string)

spam = ['apples', 'kittens', 'tofu', 'puppies']
print(items(spam))

Any feedback will be really appreciated!

Here is what the book required:

Write a function that takes a list value as an argument and returns a string with all the items separated by a comma and a space, with and inserted before the last item. For example, passing the previous spam list to the function would return 'apples, bananas, tofu, and cats'. But your function should be able to work with any list value passed to it.

Answer 1

The problem is pretty straightforward, and your solution is as well. I have only a couple of small comments:

1. The pop method in Python removes and returns the last item in a list, which can replace two lines.
2. You don't have to new_string = '' when you set new_string on the next line. As a result, things can be shortened a bit.
3. Personally, since str1 is only used once, I would personally just do it inline and not save it to a variable. The only time I save something to a variable, if it is only used once, is to shorten an otherwise-long line.
4. Concatenating two hard-coded strings should be put in one operation (i.e. ' and' + ' ' => ' and '
5. Guards! They're the best way to check input before executing.

My recommended version:

def items(some_list):
    if not some_list:
        return ''

    string_end = some_list.pop()
    if not some_list:
        return string_end

    and_display = ' and ' if len(some_list) == 1 else ', and '
    new_string = ', '.join(some_list) + and_display + string_end
    return new_string

print(items(['apples', 'kittens', 'tofu', 'puppies']))

This works fine as long as you are working with a list of strings. It will crash if you pass in an int or something that can't auto-convert to a string. That may or may not be a problem, depending on your perspective. Trying to make your code auto-convert things to strings, if they aren't strings, can be a very error-prone process. As a result, I think it is perfectly fine to write this the easy way, and let it be the developer's problem if it crashes due to non-string arguments.

Docstrings are usually helpful for this kind of thing.

Note on pass-by-reference

Hat tip to TheEspinosa: Python uses a pass-by-reference style for its arguments. As a result, modifying arguments (which is especially easy for lists, as is the case here) can result in unintentional side-effects. The use of pop (or del in your original code) results in the last entry being removed from the list, not just inside the function, but also in the variable passed in to the function. For your example usage, which calls this function with a constant value, this behavior doesn't matter. But it may as part of a larger application. There are three ways to handle this:

1. Adjust your algorithm so you don't make any changes to any arguments being passed in (@Richard Nemann's answer works that way)
2. Copy the list before using it inside your function. A simple shorthand to copy a list is: new_copy = old_copy[:]. There is also a copy module.
3. Ignore it and let it be the caller's problem.

I use all three solutions depending on the circumstances or my mood. A copy operation obviously involves some overhead, but unless you know that performance is a problem, I would still do a copy if the algorithm that avoids the need for the copy is harder to understand.

To be clear about the problem the current code can potentially introduce, try running my implementation (or yours) like this:

vals = ['apples', 'bananas', 'grapes']
items(vals)
print(len(vals))

If you run this code the last line will print 2, because in your calling scope the variable has been modified-by-reference: grapes is no longer in the list.

I said at the beginning that this was a straightforward problem. At this point in time though I think it's fair to say that the devil is in the details :)

Answer 2

Your problem can be reduced to:

def list_english(items):
    """Returns a comma-separated string of the list's items,
    separating the last item with an 'and'.
    """
    if not items:
        return ''

    if len(items) == 1:
        return items[0]

    return '{}, and {}'.format(', '.join(items[:-1]), items[-1])

Note that variable and function names in python are to be named with underscore-separated lower-case according to PEP 8.
Also consider using docstrings to state, what a function's purpose is.

if __name__ == '__main__':
    print(list_english(['apples', 'kittens', 'tofu', 'puppies']))

Also enclose your script code in an if __name__ == '__main__': block to prevent it from being run if the module is solely imported.

Finally, consider your variable naming. Python has dynamic typing, so expressive names are especially important.