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?

• Ch3steR: Your code isn't indented properly.
• greybeard: @MiguelAvila ... but for the 'and'.

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')

• Hash: Where exactly should I fix the indentation?
• Marc: @HankRyan there is an extra tab before def listsmash(listu):. I think it's due to copying and pasting the code in the question, not a big deal.
• Ch3steR: In python, str and string are two different things. Shouldn't it be str.join? Apart from that review is excellent.
• Ch3steR: And one more thing str.join with list comprehension faster than str.join with generator
• Ch3steR: From the SO answer linked The reason is that .join() needs to make two passes over the data, so it actually needs a real list. If you give it one, it can start its work immediately. If you give it a genexp instead, it cannot start work until it builds-up a new list in memory by running the genexp to exhaustion @theProgrammer