Automate the boring stuff with python - Comma Code v2

Question

This is the second version of code reviewed earlier.

Comma Code

Say you have a list value like this:

spam = ['apples', 'bananas', 'tofu', 'cats']

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.

def list_string(l_list):
 if not type(l_list) is list: # If not of type list throw's an error
 raise TypeError('Only lists are allowed')
 l_list = [str(word) for word in l_list] # Makes sure everything is a string.
 
 # Rules for Oxford comma
 if len(l_list) == 1:
 return f"'{l_list[0]}'"
 elif len(l_list) == 2:
 return f"'{l_list[0]}' and '{l_list[1]}'"
 else:
 return "'{}, and {}.'".format(', '.join(l_list[:-1]), (l_list[-1]))
 
 
def main() -> None:
 l_list = ['Apple', 'Lobster','Panda', 'Vis', 2, 3]
 output = list_string(l_list)
 print(output)
 
if __name__ == '__main__': # Program starts here, then main() get's called.
 main()

Spent 3 Hours on this one. Did look up some other code reviews; felt kinda cheating.

This time the program should do what the exercise asks.

• The Empty String Photographer: You simply need to use str(x) on each list item to make it work. Other than that, you can just copy the previous code you made.
• Reinderien: What you call cheating in this case, I call research (so long as it isn't copy-and-paste)

Answer 1

Two hard things in computer science: cache invalidation, naming, off-by one bugs.

list_string() is not a good identifier. As you go on this journey of writing code, think hard about what to name things. Feel very free to come back and rename things as it becomes clearer what they really do. Also, I have no idea what the l in l_list denotes. The name main() is conventional and is perfectly good. I like your if __name__ guard.

Consider using this signature:

def oxford_comma(items: list[str]) -> str:

You don't have to include those optional type annotations. But notice that they can do some of the heavy lifting, precisely documenting what is expected, and then you have greater freedom to create an informative name which isn't burdened by a need to reveal data types.

---

if not type(l_list) is list:
# or equivalently...
if type(l_list) is not list:

I suppose this literally fulfills the spec's contract. But a more pythonic approach, a looser more forgiving approach, would ask about isinstance(l_list, list).

That way a caller could define

class GroceryList(list):

which inherits from the base class of list. Yet we could still apply Oxford comma to it.

Also, please use $ ruff . or [flake8](https://pypi.org/project/flake8) to lint your code, and follow the advice they suggest.

---

would return 'apples, bananas, tofu, and cats'.

I fear you may have interpreted that instruction a bit too literally. I take the ' single quotes to indicate that a str value was returned, not that quotes shall be part of the result. Consider trimming the quotes.

Anyway, the comma .join() expression is very nice.

In the case of a zero-length list, [], you should probably return "" the empty string. Or perhaps raise ValueError("empty input") if you feel the caller violated the spec. Currently [] provokes an IndexError, which is not what you want.

 ... , (l_list[-1]))

Avoid decorating that last expression with extra ( ) parens, as they don't do anything.

---

Notice that you're working on a copy of the input, after you essentially assigned l_list = list(map(str, l_list)). So we are free to mutate it, without troubling the caller. Consider munging the final element, and then letting the comma .join() do most of the work:

 l_list[-1] = f'and {l_list[-1]}'

---

This code achieves its design goals.

I would be willing to delegate or accept maintenance tasks on it.

• Reinderien: For better or worse, even more Pythonic than isinstance is to duck-type and trust that what's been passed in will work.

Answer 2

Hang in there; this is about to get a bit technical.

It's a natural, and overall quite reasonable, strategy to do as you've done and write a list comprehension such as

[str(word) for word in l_list]

But what does that actually do inside of the Python interpreter? Let's ask the disassembler:

import dis
def list_string(l_list):
 if not type(l_list) is list: # If not of type list throw's an error
 raise TypeError('Only lists are allowed')
 l_list = [str(word) for word in l_list] # Makes sure everything is a string.
 # Rules for Oxford comma
 if len(l_list) == 1:
 return f"'{l_list[0]}'"
 elif len(l_list) == 2:
 return f"'{l_list[0]}' and '{l_list[1]}'"
 else:
 return "'{}, and {}.'".format(', '.join(l_list[:-1]), (l_list[-1]))
dis.dis(list_string)

 6 0 LOAD_GLOBAL 0 (type)
 2 LOAD_FAST 0 (l_list)
 4 CALL_FUNCTION 1
 6 LOAD_GLOBAL 1 (list)
 8 IS_OP 1
 10 POP_JUMP_IF_FALSE 10 (to 20)
 7 12 LOAD_GLOBAL 2 (TypeError)
 14 LOAD_CONST 1 ('Only lists are allowed')
 16 CALL_FUNCTION 1
 18 RAISE_VARARGS 1
 8 >> 20 LOAD_CONST 2 (<code object <listcomp> at 0x7f2c36c942f0, file "/home/gtoombs/petprojects/stackexchange/285474.py", line 8>)
 22 LOAD_CONST 3 ('list_string.<locals>.<listcomp>')
 24 MAKE_FUNCTION 0
 26 LOAD_FAST 0 (l_list)
 28 GET_ITER
 30 CALL_FUNCTION 1
 32 STORE_FAST 0 (l_list)
 11 34 LOAD_GLOBAL 3 (len)
 36 LOAD_FAST 0 (l_list)
 38 CALL_FUNCTION 1
 40 LOAD_CONST 4 (1)
 42 COMPARE_OP 2 (==)
 44 POP_JUMP_IF_FALSE 31 (to 62)
 12 46 LOAD_CONST 5 ("'")
 48 LOAD_FAST 0 (l_list)
 50 LOAD_CONST 6 (0)
 52 BINARY_SUBSCR
 54 FORMAT_VALUE 0
 56 LOAD_CONST 5 ("'")
 58 BUILD_STRING 3
 60 RETURN_VALUE
 13 >> 62 LOAD_GLOBAL 3 (len)
 64 LOAD_FAST 0 (l_list)
 66 CALL_FUNCTION 1
 68 LOAD_CONST 7 (2)
 70 COMPARE_OP 2 (==)
 72 POP_JUMP_IF_FALSE 50 (to 100)
 14 74 LOAD_CONST 5 ("'")
 76 LOAD_FAST 0 (l_list)
 78 LOAD_CONST 6 (0)
 80 BINARY_SUBSCR
 82 FORMAT_VALUE 0
 84 LOAD_CONST 8 ("' and '")
 86 LOAD_FAST 0 (l_list)
 88 LOAD_CONST 4 (1)
 90 BINARY_SUBSCR
 92 FORMAT_VALUE 0
 94 LOAD_CONST 5 ("'")
 96 BUILD_STRING 5
 98 RETURN_VALUE
 16 >> 100 LOAD_CONST 9 ("'{}, and {}.'")
 102 LOAD_METHOD 4 (format)
 104 LOAD_CONST 10 (', ')
 106 LOAD_METHOD 5 (join)
 108 LOAD_FAST 0 (l_list)
 110 LOAD_CONST 0 (None)
 112 LOAD_CONST 11 (-1)
 114 BUILD_SLICE 2
 116 BINARY_SUBSCR
 118 CALL_METHOD 1
 120 LOAD_FAST 0 (l_list)
 122 LOAD_CONST 11 (-1)
 124 BINARY_SUBSCR
 126 CALL_METHOD 2
 128 RETURN_VALUE
Disassembly of <code object <listcomp> at 0x7f2c36c942f0, file "/home/gtoombs/petprojects/stackexchange/285474.py", line 8>:
 8 0 BUILD_LIST 0
 2 LOAD_FAST 0 (.0)
 >> 4 FOR_ITER 6 (to 18)
 6 STORE_FAST 1 (word)
 8 LOAD_GLOBAL 0 (str)
 10 LOAD_FAST 1 (word)
 12 CALL_FUNCTION 1
 14 LIST_APPEND 2
 16 JUMP_ABSOLUTE 2 (to 4)
 >> 18 RETURN_VALUE

code object <listcomp> is referenced, and then later defined, to be an inner function - even though you haven't explicitly asked for an inner function to be made. In fact, for the most common implementations of Python, this is how generators are created.

It's not the end of the world for a function like this to be created, but if a function needs to be created, we should replace it with our own explicit function that

• does everything we want it to without a pre-processing step: str()ing the items, yielding commas and yielding and;
• avoids the in-memory list that you have created, going straight from a generator to a join; and
• doing it all in a procedural style that is very easy to understand.

On top of that, this is a golden opportunity to write unit tests, so do that.

The generator approach can look like

from typing import Sequence, Any, Iterator
def comma_and_insertion(items: Sequence[Any]) -> Iterator[str]:
 match items:
 case (only,):
 yield str(only)
 case first, second:
 yield from (str(first), ' and ', str(second))
 case *others, second_last, last:
 for item in others:
 yield str(item)
 yield ', '
 yield from (str(second_last), ', and ', str(last))
def list_string(items: Sequence[Any]) -> str:
 """Format with Oxford commas and the word "and" """
 return ''.join(comma_and_insertion(items))
def test() -> None:
 assert list_string([]) == ''
 assert list_string(['foo']) == 'foo'
 assert list_string(['foo', 'bar']) == 'foo, and bar'
 assert list_string(
 ['Apple', 'Lobster', 'Panda', 'Vis', 2, 3]
 ) == 'Apple, Lobster, Panda, Vis, 2, and 3'
if __name__ == '__main__':
 test()

If we dis() our new inner function comma_and_insertion, we get something similar to your implicit listcomp (note the FOR_ITER) - but that takes on all of the work in one pass:

 0 GEN_START 0
 5 2 LOAD_FAST 0 (items)
 6 4 DUP_TOP
 6 MATCH_SEQUENCE
 8 POP_JUMP_IF_FALSE 19 (to 38)
 10 GET_LEN
 12 LOAD_CONST 1 (1)
 14 COMPARE_OP 2 (==)
 16 POP_JUMP_IF_FALSE 19 (to 38)
 18 UNPACK_SEQUENCE 1
 20 STORE_FAST 1 (only)
 22 POP_TOP
 7 24 LOAD_GLOBAL 0 (str)
 26 LOAD_FAST 1 (only)
 28 CALL_FUNCTION 1
 30 YIELD_VALUE
 32 POP_TOP
 34 LOAD_CONST 0 (None)
 36 RETURN_VALUE
 6 >> 38 POP_TOP
 8 40 DUP_TOP
 42 MATCH_SEQUENCE
 44 POP_JUMP_IF_FALSE 45 (to 90)
 46 GET_LEN
 48 LOAD_CONST 2 (2)
 50 COMPARE_OP 2 (==)
 52 POP_JUMP_IF_FALSE 45 (to 90)
 54 UNPACK_SEQUENCE 2
 56 STORE_FAST 2 (first)
 58 STORE_FAST 3 (second)
 60 POP_TOP
 9 62 LOAD_GLOBAL 0 (str)
 64 LOAD_FAST 2 (first)
 66 CALL_FUNCTION 1
 68 LOAD_CONST 3 (' and ')
 70 LOAD_GLOBAL 0 (str)
 72 LOAD_FAST 3 (second)
 74 CALL_FUNCTION 1
 76 BUILD_TUPLE 3
 78 GET_YIELD_FROM_ITER
 80 LOAD_CONST 0 (None)
 82 YIELD_FROM
 84 POP_TOP
 86 LOAD_CONST 0 (None)
 88 RETURN_VALUE
 8 >> 90 POP_TOP
 10 92 MATCH_SEQUENCE
 94 POP_JUMP_IF_FALSE 84 (to 168)
 96 GET_LEN
 98 LOAD_CONST 2 (2)
 100 COMPARE_OP 5 (>=)
 102 POP_JUMP_IF_FALSE 84 (to 168)
 104 EXTENDED_ARG 2
 106 UNPACK_EX 512
 108 STORE_FAST 4 (others)
 110 STORE_FAST 5 (second_last)
 112 STORE_FAST 6 (last)
 11 114 LOAD_FAST 4 (others)
 116 GET_ITER
 >> 118 FOR_ITER 10 (to 140)
 120 STORE_FAST 7 (item)
 12 122 LOAD_GLOBAL 0 (str)
 124 LOAD_FAST 7 (item)
 126 CALL_FUNCTION 1
 128 YIELD_VALUE
 130 POP_TOP
 13 132 LOAD_CONST 4 (', ')
 134 YIELD_VALUE
 136 POP_TOP
 138 JUMP_ABSOLUTE 59 (to 118)
 14 >> 140 LOAD_GLOBAL 0 (str)
 142 LOAD_FAST 5 (second_last)
 144 CALL_FUNCTION 1
 146 LOAD_CONST 5 (', and ')
 148 LOAD_GLOBAL 0 (str)
 150 LOAD_FAST 6 (last)
 152 CALL_FUNCTION 1
 154 BUILD_TUPLE 3
 156 GET_YIELD_FROM_ITER
 158 LOAD_CONST 0 (None)
 160 YIELD_FROM
 162 POP_TOP
 164 LOAD_CONST 0 (None)
 166 RETURN_VALUE
 10 >> 168 POP_TOP
 170 LOAD_CONST 0 (None)
 172 RETURN_VALUE

• FMc: 'foo, and bar' ?
• Jarne Vercruysse: if len(items) > 2: Are u not missing that statement between yield str(item) and yield ', '. To only place a comma with 3 items in a list? How do you mean golden opportunity?
• Reinderien: That's not in the problem statement, but it is in the OP's code so okay

Answer 3

Naming: prefer specific over generic. The function name tells us nothing about its behavior or purpose. Better alternatives could mention the idea of joining, punctuation, or the Oxford comma.

Naming: prefer ordinary over awkward. The function's argument could be anything, so how do we name it? Not with something like l_list: it's needlessly awkward to type, visually off-putting, and tells us nothing more specific than several ordinary or conventional alternatives: items, vals, xs, etc.

Duck typing: Python generally embraces it. Whenever feasible, avoid imposing overly strict typing requirements on functions and methods. We don't actually care whether the function's argument is a list or even a sequence. Any iterable of values convertible to str will work. Many data types satisfy our needs: list[str], tuple[str], a dict with str keys, just a str (of characters), or endless user-defined objects.

Unnecessary quote marks. Your current code is adding single-quote marks to the returned string that I don't think are called for in the problem statement.

Simplifying the logic: fewer conditional branches. If there are 0, 1, or 2 values, you can just join them with ' and '. Otherwise, you do the Oxford comma routine -- which can be made more readable by popping off the last value.

Writing better demo code. It's great that you posted your question with a main() method to exercise your code. Even better would be to do the same thing covering the primary input scenarios: 0 values, 1, 2, 3, and 4+. An illustration is shown below. Better still would be a demo framed as a test with assertions: I was too lazy for that, but we have higher expectations for you!

def demo():
 vals = ['A', 'B', 'C', 99]
 for i in range(len(vals) + 1):
 xs = vals[0:i]
 text = oxford_comma_join(xs)
 print(xs, text)
def oxford_comma_join(xs):
 # Convert iterable to sequence of str.
 try:
 seq = [str(x) for x in xs]
 except Exception:
 raise ValueError
 # Return joined str.
 if len(seq) < 3:
 return ' and '.join(seq)
 else:
 last = seq.pop()
 return ', '.join(seq) + ', and ' + last
if __name__ == '__main__':
 demo()