Rock paper scissors in python with "algorithm"

[deanhystad]

I get that you have "constraints". I don't. I'm going to post what I think reads well. I don't care that you can't use it directly. Actually I don't want to provide answers that you can use directly. Even though this is not in the homework section it is obviously homework, and we are not supposed to do homework assignments.

Are you "constrained" to using bad variable names? "most_frequent_recent_computer_action" is a really bad variable name. First off, it is incorrect. History SHOULD be a list of user actions, not computer actions, so a better name is "most_frequent_recent_user_action". That is a more accurate variable name, but it is still a poor variable name.

After you get you AI working you will play some games and maybe change you mind about how the computer makes choices. Maybe you will change only using recent user actions to using all user actions. Now that really long variable name is misleading again, so you change it to "most_frequent_user_action". You make the change and you still don't like how it plays, so you decide to add in some randomness. The variable name is misleading again, so you change it to "predicted_user_action". This is finally a good variable name, and the name you should have used from the beginning. The name conveys what the variable is, not how the variable value was computed.

Choose variable and function names that describe their purpose, not their implementation.

Not only was "most_frequent_recent_computer_action" incorrect, restrictive and not all that informative, it was too long. Really long variable and function names make code hard to read. This code is hard to read.

        most_frequent_recent_computer_action = \
            GameAction(mode(user_actions_history[-NUMBER_RECENT_ACTIONS:]))

This is easier to read:

recent_history = user_actions_history[-NUMBER_RECENT_ACTIONS:]
predicted_user_action = GameAction(mode(recent_history))

Even that is a bit wordy. Why mention the user at all?

prediction = mode(user_actions_history[-NUMBER_RECENT_ACTIONS:]
prediction = GameAction(prediction)  # May not be necessary

Either one looks much better than the original.

A couple other things.

Why do this:

    computer_selection = random.randint(0, len(GameAction) - 1)
    computer_action = GameAction(computer_selection)

when you can do this

computer_action = random.choice(list(GameAction))

Ideally you would like to call random.choice(GameAction), but GameAction is not "indexable". If you are going to use enums, really use the enum instead of doing most things with ints.

Both of these statements can raise an exception.

    user_selection = int(input(f"\nPick a choice ({game_choices_str}): "))
    user_action = GameAction(user_selection)

There is a try/except in main, but it really belongs next to the code that generates the exception. I don't know if you can move it, but that is really where the belongs.

Not a rule, but a really strong suggestion: You should not have print statements in functions unless the purpose of the function is to do output. This print statement does not belong in "computer_action().

print(f"Computer picked {computer_action.name}.")

I think you already ran into this problem. I remember you mentioning that you had to remove a print somewhere to prevent something being printed twice. I think you should go further and remove all prints except in one method/function that is dedicated to printing the results. As your code is currently written the print in computer_action() it should be moved to assess_game() which does all the other game results printing. Having a print statement in computer_action() makes it so others cannot reuse your code. In my example I also made assess_game() a pure function (function with no side effects like printing to stdout). Another programmer could easily reuse my code as the logic for a GUI based rock, paper, scissors game. They would provide their own "main()" and "get_uer_action()" functions.