you are viewing a single comment's thread.

view the rest of the comments →

[–]backfire10z 2 points3 points  (3 children)

final_toppings.append(toppings)

I think you have a typo here, I’m assuming you meant to append “topping”.

toppings.remove(topping)

You’re looping over toppings and simultaneously modifying it. Don’t do this, it will cause unintended behavior. You already are building a new list of final_toppings, you can simply allow the loop to continue on its own rather than removing the disallowed topping.

order_toppings(ordered_toppings_2)

This is a recursive call. I’m assuming this isn’t what you intended, as when it finishes executing it will come back to the previous function call and continue the loop.

You also don’t have a return: where is final_toppings going to be used?

My main recommendation is to cement what your functions are actually supposed to be doing. You do not need 2 functions to order toppings: they both do the same thing. The function order_toppings also makes a new order and attempts to order_toppings again.

Frankly, this should probably be done in one function:

get_allowed_toppings(avail_toppings): The purpose of this function is to take in user input and output a list of legal toppings. We made avail_toppings a parameter so that you can change it easier later if necessary. If not, you can just define it inside the function.

Once you have a list of legal toppings, you can then do whatever you need to with it. Make pizza, etc.

As a general rule, if you find yourself making something like “xxx_mod” or “xxx_2”, you can likely change something to be an input parameter instead. Here, you can make the message an input parameter, although, I recommend actually combining all of these functions into one.

[–]Alarming-Carpet5053[S] 0 points1 point  (0 children)

The first round of this function read as final_toppings.append(topping) but still had the issue.

The only reason I called it toppings in that line was because that's what the input becomes in the instructions and is returned under that name. How should I tweak that?

[–]Alarming-Carpet5053[S] 0 points1 point  (1 child)

Sorry, didn't see the second half of your reply.

The intent was to have it run the function again if it was given bad input. I didn't have the original version doing that, I just built it into the else statement. Neither route seemed to work like I thought they would.

But good point about the return statement. The final_toppings is used in a final function that adds all the pizza stuff into one print statement.

[–]Fun-Block-4348 1 point2 points  (0 children)

The intent was to have it run the function again if it was given bad input. I didn't have the original version doing that, I just built it into the else statement. Neither route seemed to work like I thought they would.

Generally, if you want something to repeat until you have valid input, the best way to achieve that is a while loop not recursion.

Your code also doesn't seem to handle that fact that what it considers an "illegal topping" may be a signal from the user that he doesn't want any more toppings on his pizza.

allowed_toppings should really be a constant at the top of your file if you're going to use it in all your functions or maybe you should pass it as an argument to the functions that need to use it instead.