Code to take a list, convert to string and insert comma between the 'elements' - From Automate The Boring Stuff

Question

I have been working on this question in particular. I'm a bit of a python beginner myself, having re-learnt functions just a few days back.

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. Be sure to test the case where an empty list [] is passed to your function.

My idea for this was to have my function run a for loop on the range of the number of elements of the list, add the elements of the list to a comma. If the element of the list happens to be the last element, 'and' and that specific element are added together.

Here is the code:

def listsmash(listu):
    y = ''
    for x in range(len(listu)):
        if x == len(listu) -1 :
            y+= 'and '+ str(listu[x])
        else:
            y+= str(listu[x]) + ','
    return y    
print(listsmash([1,345,34656,456456,456454564]))

Is this an efficient piece of code? Any way for me to make it more efficient than it already is?

Answer 1

Nice solution, these are my suggestions:

• Indentation: the first line is not indented correctly, there is an extra tab or spaces before def listsmash(listu):.
• Code formatting: follow PEP 8 to format the code. You can use pep8online or just use the auto-formatting feature of many IDEs.
• Bug: the output should be apples, bananas, tofu, and cats but the code outputs apples,bananas,tofu,and cats. There should be a space after the comma.
• Edge case: if the input list contains only apples the code outputs and apples. This is an edge case to handle, probably at the beginning of the method.
• Performance: there is no much to worry about performances, as it runs linearly to the input size. However, the first if condition is executed everytime:

  for x in range(len(listu)):
      if x == len(listu) -1 :
          y+= 'and '+ str(listu[x])
      else:
          y+= str(listu[x]) + ','
  

  Limit the number of iterations to len(listy) - 1 and add the last element outside the for-loop:

  def listsmash(listu):
      y = ''
      for x in range(len(listu) - 1):
          y += str(listu[x]) + ', '
      y += 'and ' + str(listu[-1])
      return y
  

• Use str.join: str.join is a handy function in this case:

  def listsmash(listu):
      y = ', '.join(str(value) for value in listu[:-1])
      return y + ', and ' + str(listu[-1])
  

  It joins all elements of listu (except the last one) with , and then we can append the last element at the end. Additionally, the function .join() is faster than +=.
• Naming: I am not sure what listu means, maybe you can consider a better name like items following from the problem description.

Runtime for 1 million items:

Original: 4.321 s
Improved: 0.418 s

Full code:

from random import randint
from time import perf_counter


def original(listu):
    y = ''
    for x in range(len(listu)):
        if x == len(listu) - 1:
            y += 'and ' + str(listu[x])
        else:
            y += str(listu[x]) + ', '
    return y


def improved(items):
    result = ', '.join(str(item) for item in items[:-1])
    return result + ', and ' + str(items[-1])


# Correctness
items = ['apples', 'bananas', 'tofu']
assert original(items) == improved(items)

# Benchmark
n = 1_000_000
items = [randint(0, n) for _ in range(n)]

t0 = perf_counter()
original(items)
total_time = perf_counter() - t0
print(f'Original: {round(total_time, 3)} s')

t0 = perf_counter()
improved(items)
total_time = perf_counter() - t0
print(f'Improved: {round(total_time, 3)} s')