Hfrhyu

This is based off the popular Pokemon Turn Based project (here).

GOAL

Write a simple game that allows the user and the computer to take
turns selecting moves to use against each other. Both the computer and
the player should start out at the same amount of health (such as
100), and should be able to choose between the three moves:

• The first move should do moderate damage and has a small range (such as 18-25).




• The second move should have a large range of damage and can deal high or low damage (such as 10-35).




• The third move should heal whoever casts it a moderate amount, similar to the first move.

After each move, a message should be printed out that tells the user
what just happened, and how much health the user and computer have.
Once the user or the computer's health reaches 0, the game should end.

SUBGOALS

• When someone is defeated, make sure the game prints out that their health has reached 0, and not a negative number.




• When the computer's health reaches a set amount (such as 35%), increase it's chance to cast heal.




• Give each move a name.

I am new to Python and I am trying to use OOP. Did I use my classes properly and is there a way to reduce the number of if statements in my code? Since this is my first code review, how is the formatting, number of comments, etc?

import random

class Pokemon:

 """Blueprint for turn based Pokemon battle"""

 def __init__(self, attack_choice):

 self.__attack_choice = attack_choice


 def attack(self):

 if self.__attack_choice == 1:
 attack_points = random.randint(18,25)
 return attack_points

 elif self.__attack_choice == 2:
 attack_points = random.randint(10,35)
 return attack_points

 else:
 print("That is not a selection. You lost your turn!")

 def heal(self):

 heal_points = random.randint(18,25)
 return heal_points 

###########################################################################

user_health = 100
mew_health = 100
battle_continue = True

while battle_continue == True:
 print("nATTACK CHOICESn1. Close range attackn2. Far range attackn3. Heal")
 attack_choice = eval(input("nSelect an attack: "))

 # Mew selects an attack, but focuses on attacking if health is full. 
 if mew_health == 100:
 mew_choice = random.randint(1,2)

 else:
 mew_choice = random.randint(1,3)

 mew = Pokemon(mew_choice)
 user_pokemon = Pokemon(attack_choice)

 # Attacks by user and Mew are done simultaneously.
 if attack_choice == 1 or attack_choice == 2:
 damage_to_mew = user_pokemon.attack()
 heal_self = 0
 print("You dealt",damage_to_mew,"damage.")

 if mew_choice == 1 or mew_choice ==2:
 damage_to_user = mew.attack()
 heal_mew = 0
 print("Mew dealt", damage_to_user, "damage.")

 if attack_choice == 3:
 heal_self = user_pokemon.heal()
 damage_to_mew = 0
 print("You healed",heal_self,"health points.")

 if mew_choice == 3:
 heal_mew = mew.heal()
 damage_to_user = 0
 print("Mew healed", heal_mew, "health points.")

 user_health = user_health - damage_to_user + heal_self
 mew_health = mew_health - damage_to_mew + heal_mew

 # Pokemon health points are limited by a min of 0 and a max of 100.
 if user_health > 100:
 user_health = 100

 elif user_health <= 0:
 user_health = 0
 battle_continue = False

 if mew_health > 100:
 mew_health = 100

 elif mew_health <= 0:
 mew_health = 0
 battle_continue = False

 print("Your current health is", user_health)
 print("Mew's current health is", mew_health)

print("Your final health is", user_health)
print("Mew's final health is", mew_health)

if user_health < mew_health:

 print("nYou lost! Better luck next time!")

else:

 print("nYou won against Mew!")

You have chosen a great problem to begin with, however there are a few things you get wrong about OOP. OOP is not simply about using a class for "everything", it's a kind of way to think about a problem.

To begin with, it helped me a lot to think about a class as a prototype of a "thing". In your case the "thing" would be a Pokemon. A Pokemon can do certain things. In your simplified versions that would be 1. attack another Pokemon and 2. heal itself. Often these actions are reflected in the classes's methods. I think you mostly understood that. What else is there about the Pokemon/"thing" it has certain properties that describe it. I would say that's an aspect you did not think about. A property could be name, color, ... or it's health status. We also learn that the health can only be between 0 and 100.

So with this on our mind, let's think of a new design for class Pokemon:

class Pokemon:
 """Blueprint for a new pokemon"""

 def __init__(self):
 self._health = 100
 # ^--- the leading _ is a convention to mark internal values

 @property
 def health(self):
 """The health of the Pokemon which is between 0 and 100"""
 return self._health

 @health.setter
 def health(self, new_health):
 """Set the new heath value"""
 # here the health limits are enforced
 self._health = min(100, max(0, new_health))

 def attack(self, other, choice):
 """Attack another Pokemon with the chosen attack (1 or 2)

 This function also returns the raw amount of random damage dealt. The
 amount of damage dealt depends on the attack type.
 """
 if choice == 1:
 attack_points = random.randint(18, 25)
 elif choice == 2:
 attack_points = random.randint(10, 35)
 else:
 print("That is not a selection. You lost your turn!")
 attack_points = 0
 other.health -= attack_points
 return attack_points

 def heal(self):
 """Heal the Pokemon"""
 heal_points = random.randint(18, 35)
 self.health += heal_points
 return heal_points

A lot of this should look familiar to you, since the main work is still done by the code you've written. What is new that the Pokemon now has health. If your unfamiliar with properties in Python just think of them as synthatic sugar on what would normally be done using getter and setter functions in other languages. There is a great SO post with a nice explanation on properties. In addition to that, attack and heal now handle the update of the health value for the Pokemon, as well as the opponent. This allows us to write up a simple battle in a very concise way:

mew = Pokemon()
user = Pokemon()
mew.attack(user, 1)
print(f"User health after attack: user.health")
user.heal()
print(f"User health after heal: user.health")
mew.heal() # health should still only be 100
print(f"Mew's health after 'illegal' heal: user.health")

which prints for example:

User health after attack: 75
User health after heal: 100
Mew's health after 'illegal' heal: 100

No additional variables that need to track health status, no checking on the health limits. Everything is nicely encapsulated into the Pokemon class. As DaveMongoose pointed out in his comment, a drawback of this approach is that the Pokemon can not be defeated as long as it heals after an attack, no matter how much damage it took.

Short break on style and other conventions

Another thing that has changed in contrast to your solution is the documentation I added to each function. Python has an "official" Style Guide (which is worth a read on its own) with a section on how to write these so called docstrings here.

It also features guidelines on where to use blank lines and where not to use them. In my oppinion the excessive use of blank lines in your code hinders readability more than it does help to structure the code.

I also used only a single leading underscore for my internal value instead of two as you did. The Style Guide also has you covered on this topic. In general you should always use a single leading underscore to mark functions and variables/members as internal. For more details on what happens if you use two leading underscores, follow the link above.

After that short intermezzo, let's look on how the battle simulation looks with the new class:

def battle_simulation():
 """Run a simple interactive Pokemon battle simulation"""
 mew = Pokemon()
 user_pokemon = Pokemon()
 while True:
 print("nATTACK CHOICESn1. Close range attackn2. Far range attackn3. Heal")
 attack_choice = int(input("nSelect an attack: "))
 # DON'T use eval on user input, this can be dangerous!

 # Mew selects an attack, but focuses on attacking if health is full.
 mew_choice = random.randint(1, 2 if mew.health == 100 else 3)
 # this is your original distinction just condensed into a single line

 # Attacks by user and Mew are done simultaneously
 # with the changes to Pokemon, there is no need to save all the
 # intermediate damage/heal values -> nice and short code
 if attack_choice != 3:
 print(f"You dealt user_pokemon.attack(mew, attack_choice) damage.")

 if mew_choice != 3:
 print(f"Mew dealt mew.attack(user_pokemon, mew_choice) damage.")

 if attack_choice == 3:
 print(f"You healed user_pokemon.heal() health points.")

 if mew_choice == 3:
 print(f"Mew healed mew.heal() health points.")

 if mew.health == 0 or user_pokemon.health == 0:
 break

 print(f"Your current health is user_pokemon.health")
 print(f"Mew's current health is mew.health")

 print(f"Your final health is user_pokemon.health")
 print(f"Mew's final health is mew.health")

 if user_pokemon.health < mew.health:
 print("nYou lost! Better luck next time!")
 else:
 print("nYou won against Mew!")


if __name__ == "__main__":
 battle_simulation()

As you can see, we got rid of all the variables needed to store damage and heal values since everything is no done internally in the Pokemon class. With this, the code becomes quite a bit shorter, clearer and easier to read. Cool, isn't it?

A thing you will see me use quite often are the so called f-strings. They take arbitrary Python expressions (function calls, variables, ...) and allow to incorporate them into a formatted string. If you want to know more about them, I would recommend this blog post.

Happy Coding!

The heal method and the handling of health points strike me as odd. In the current setup,

• heal does not heal; it simply returns a random number. It also currently has no direct relation to any part of the Pokemon class, so it doesn't really make sense as a method of the class.




• The health points of each Pokemon are "loose" in the script, along with instances of the Pokemon class. You have a Pokemon class representing a Pokemon, and it makes sense that health would be an attribute of a Pokemon itself.

The design can be improved by doing something like (stripped down):

class Pokemon:
 def __init__(self, start_health):
 self.hp = start_health

 def heal(self, heal_amount):
 self.hp += heal_amount

 def hurt(self, damage):
 self.hp -= damage

Now, you can do something like:

mew = Pokemon(100)
mew.hurt(50) # Ow
mew.heal(49) # Back to almost full
print(mew.hp) # Prints 99

And you don't need to have loose health values floating around in the code for each Pokemon in use. Using the method like this also lets you check the health after healing to ensure that you didn't exceed the maximum health allowed. As long as you're going to make use of a class, you can benefit by having it encapsulate all the data (like health) that's directly relevant to it.

I also decided to not have heal generate the random amount to heal for the following reasons:

• Are you sure that every time you want to heal you want it to be a random value, and are you sure you will always want it to be a random value in the range (10, 25]? As you mentioned, what about potions? Are you sure you want potion healing to be random, and always in that range? What about when Pokemon level up and have more health? Are you sure that you'll only want to heal them in that narrow range? The Pokemon class simply does not have enough information to always decide how much Pokemon should be healed by, and that shouldn't be its responsibility in the first place anyways.



• 
  

  Using random data complicates testing. Say you want to add tests to ensure the correctness of the hurt and heal methods. You would expect this to always pass:

  
  
  

  start_health = 100
  poke = Pokemon(start_health)
  poke.hurt(50)
  poke.heal(50)
  
  # A simplified test
  print("Test Passed?", poke.hp == start_health)
  

  
  
  

  If the methods are correct, you would expect this test to always pass. With random data though, your tests can't be as definitive. You may know that it was hurt by some value, then healed by some value, but that isn't enough information to ensure correct functionality (unless you're only testing that it produces results in a certain range).

  
  
  

  For small toy projects like this, testing isn't really necessary (although it's always good to practice). When you start dealing with larger projects split across multiple files, written by multiple people, and changing code that you haven't looked at in potentially months, you need tests to ensure that code stays correct from one change to the next. Making testing easier helps ensure the validity of the tests, and prevents you from wasting your time trying to Jerry-rig some test in place after the fact.

  
  

If some code wants to hurt the Pokemon by a random amount, they can generate the random data themselves to pass in to hurt.

while battle_continue == True: has a redundant condition. while (and if) just check if their condition is truthy. You can just write:

while battle_continue: