I do not have an intention to ask for an answer, but I want to know if my answer is good for a two-week Python beginner?

Question: (Automate the Boring Stuff with Python/ page 102)

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.

My Answer:

spam = ['apples', 'bananas', 'tofu', 'cats']
def sentence(i):
    for n in range(0, len(spam) - 1):
        print(spam[n], end=', ')
    print('and ' + spam[-1])
  • 1
    \$\begingroup\$ Your function is not actually "able to work with any list value passed to it., because it relies on the globally defined list spam and not the argument it is passed (which you named i). \$\endgroup\$ – Graipher Mar 30 at 13:22
  • 1
    \$\begingroup\$ Moreover, the implementation is incorrect, an empty list (spam = []) will result in an exception: IndexError: list index out of range. \$\endgroup\$ – Anonymous Mar 30 at 14:18
  • \$\begingroup\$ Moreover, your function returns None instead of the string specified. \$\endgroup\$ – greybeard Mar 30 at 16:41

Your function isn't returning anything. You should be building a string and returning it from your function. str.join() will be useful for building your string, return it with the return keyword.

| improve this answer | |
  • \$\begingroup\$ After reviewing my code, I have realized that my function won't work if I use another list. I will consider your feedback so as to rewrite my program. Thank you for letting me know the errors in my code. \$\endgroup\$ – khanghong Apr 3 at 2:21

Since testing falls under the general heading of code review, I'm just going to review your testing strategy.

The question says:

your function should be able to work with any list value passed to it.

but you are not testing this; you're only testing the single example that was given to you (and which, as it happens, you've hard-coded into your function). Here's how you could test it:

print(sentence(['eggs', 'bacon', 'spam', 'bacon']))

Does this print the string eggs, bacon, spam, and bacon? If not, why? (Try to understand the bug before you fix it -- don't just change things randomly until it works!)

An even better test would be:

assert sentence(spam) == "apples, bananas, tofu, and cats"
assert sentence(['eggs', 'bacon', 'spam', 'bacon']) == "eggs, bacon, spam, and bacon"

Add this to your program, and then fix the sentence function so that neither line of code causes your program to fail. This is called "test driven development" -- write the test first that says how you want the function to work, then write the function until the test passes!

| improve this answer | |
  • \$\begingroup\$ I am grateful to your reply. I have realized that my program, as you said, have been hard-coded with the spam list. After reading your review, I will check my code again with your two assert lines. Once again, thank you for letting me know what to do next. \$\endgroup\$ – khanghong Apr 3 at 2:17

Not the answer you're looking for? Browse other questions tagged or ask your own question.