Text based fight game

Question

I learned code about a week ago and I wanna hear some recommendations and stuff I could add to my code. This is a fight system I made for my text based game. Please tell me what I could do better.

import random
import time
import sys
def fight_snake():
    fdmg1 = int(random.randint(1,7))
    edmg1 = int(random.randint(1,6))
    print("You hit a", fdmg1)
    print("The Snake hits a", edmg1)
    time.sleep(1)
    if edmg1 > fdmg1:
        print("You died")
    elif edmg1 < fdmg1:
        print("You killed the Snake")
    else:
        print("You didnt kill the Snake but you managed to escape")

def fight_enemy_a():
    fdmg1 = int(random.randint(1,7))
    edmg_a = int(random.randint(4,10))
    print("You hit a", fdmg1)
    print("The Enemy hits a", edmg_a)
    time.sleep(1)
    if edmg_a > fdmg1:
        print("You died")
    elif edmg_a < fdmg1:
        print("You killed the Enemy")
    else:
        print("You didnt kill the Enemy but you managed to escape")

def fight_enemy_b():
    fdmg1 = int(random.randint(1,7))
    edmg_b = int(random.randint(4,9))
    print("You hit a", fdmg1)
    print("The Enemy hits a", edmg_b)
    time.sleep(1)
    if edmg_b > fdmg1:
        print("You died")
    elif edmg_b < fdmg1:
        print("You killed the Enemy")
    else:
        print("You didnt kill the Enemy but you managed to escape")

def fight_enemy_c():
    fdmg1 = int(random.randint(1,7))
    edmg_c = int(random.randint(1,7))
    print("You hit a", fdmg1)
    print("The Enemy hits a", edmg_c)
    time.sleep(1)
    if edmg_c > fdmg1:
        print("You died")
    elif edmg_c < fdmg1:
        print("You killed the Enemy")
    else:
        print("You didnt kill the Enemy but you managed to escape")

Answer 1

Design

As it stands, your current design is very rigid, and not extensible in any way. The best approach for a situation like this could be to use Object Oriented Programming, but in accordance with the Python talk, Stop Writing Classes, all of your code can be shortened to one function.

def fight_enemy(enemy_name, min_enemy_damage, max_enemy_damage, min_player_damage, max_player_damage):
    enemy_damage_dealt = random.randint(min_enemy_damage, max_enemy_damage)
    player_damage_dealt = random.randint(min_player_damage, max_player_damage)

    if enemy_damage_dealt > player_damage_dealt:
        print("Uh-oh! You died!")
    elif enemy_damage_dealt < player_damage_dealt:
        print("You killed the {enemy_name}".format(enemy_name=enemy_name))
    else:
        print("You walk away unscathed, but the {enemy_name} still lives.".format(enemy_name=enemy_name))

Here's an example function call:

fight_enemy("Enemy", 1, 10, 1, 10)

---

Naming

Many of your variable names are so cryptic that it's understand what they're doing, even in the context of other code. Here's a small list of some of your bad variable names.

• fdmg1
• edmg_c

While identifying bad names is important, fixing them is even better. Here's a small list of tips to follow when naming variables.

• Don't make the name too short. (Unless you're doing stuff like coordinates, with x, and y.)
• Don't include abbreviations unless the name is long, and the abbreviations are still readable.
• Use underscores _ to separate words.

---

Nitpicks

When getting an value from random.randint(), there's no need to convert to an integer using int. random.randint() already returns an integer value.

You should also be using string formatting with str.format, rather than passing multiple arguments to print(). For example, the following line:

print("The Enemy hits a", edmg_b)

Would become:

print("The Enemy hits a {damage}".format(damage=edmg_b))

For more on str.format, see the docs.

You also have to useless imports:

import time
import sys

If you don't use the contents of these modules, the imports can be removed.

---

In short, I've refactored your code down to this:

import random

def fight_enemy(enemy_name, min_enemy_damage, max_enemy_damage, min_player_damage, max_player_damage):
    enemy_damage_dealt = random.randint(min_enemy_damage, max_enemy_damage)
    player_damage_dealt = random.randint(min_player_damage, max_player_damage)

    if enemy_damage_dealt > player_damage_dealt:
        print("Uh-oh! You died!")
    elif enemy_damage_dealt < player_damage_dealt:
        print("You killed the {enemy_name}".format(enemy_name=enemy_name))
    else:
        print("You walk away unscathed, but the {enemy_name} still lives.".format(enemy_name=enemy_name))

Hope this helps! Enjoy!

Answer 2

DRY

You are repeating yourself in a lot of places in your code. In fact, I'd say most all of your code is just a giant repeat.

---

You have 4 functions in your code. These functions are almost exactly identical, except for a few tiny points.

Each function is doing the same thing: getting two random numbers, logging them out with a message, and then running some conditionals on these numbers to determine what other message should be sent.

Therefore, why don't you stick these all into one function?

---

To fix your code, let's start by taking one of the functions and working our way towards having non-repeating code.

The function we will work from will be fight_snake:

def fight_snake():
    fdmg1 = int(random.randint(1,7))
    edmg1 = int(random.randint(1,6))
    print("You hit a", fdmg1)
    print("The Snake hits a", edmg1)
    time.sleep(1)
    if edmg1 > fdmg1:
        print("You died")
    elif edmg1 < fdmg1:
        print("You killed the Snake")
    else:
        print("You didnt kill the Snake but you managed to escape")

I have one main problem with this function: you are hard-coding the enemy to be a "snake".

What if you want to have another enemy? Are you going to create another practically identical function just to print out a word that is not "snake"? Of course not; that would not be very effective.

To make this function more flexible, let's add a parameter to the function that is the enemy that the user is fighting.

That is done simply like this:

def fight_enemy(enemy):

Note: I changed the function name to fight_enemy because the function is no longer specific to a snake.

Now, instead of using the string "snake", we can use the parameter that was passed in to the function.

def fight_enemy(enemy):
    fdmg1 = int(random.randint(1,7))
    edmg1 = int(random.randint(1,6))
    print("You hit a", fdmg1)
    print("The " + enemy + " hits a", edmg1) # <== Changed
    time.sleep(1)
    if edmg1 > fdmg1:
        print("You died")
    elif edmg1 < fdmg1:
        print("You killed the " + enemy) # <== Changed
    else:
        print("You didnt kill the " + enemy + " but you managed to escape") # <=== Changed

Now our function is looking pretty good, but there is still one problem: the random numbers. For each of your 4 functions, you generated different random numbers.

Now you mentioned that you are very new to this, so this next section may be very confusing.

To be able to implement different random numbers, let's create a dictionary that maps a single value to a tuple of two values for creating random numbers.

That could be done like this:

random_numbers = {
    'a': ((1,7),(1,6)),
    'b': ((1,7),(4,10)),
     ...
}

Note: random_numbers and 'a','b',... are bad variable names. I used these because I was unsure of their original use in the code. You should think of better ones.

Great, we have a dictionary, but that still doesn't help us not have to repeat code. That means it's time to create more parameters!

This parameter will be one of the keys of the random_numbers dictionary so the function knows which random number to grab.

def fight_enemy(enemy, type):
    fdmg1 = int(random.randint(1, 7))
    edmg1 = int(random.randint(random_numbers[type][0], random_numbers[type][1]))
    print("You hit a", fdmg1)
    print("The " + enemy + " hits a", edmg1)
    time.sleep(1)
    if edmg1 > fdmg1:
        print("You died")
    elif edmg1 < fdmg1:
        print("You killed the " + enemy)
    else:
        print("You didnt kill the " + enemy + " but you managed to escape")

There! Now, when calling this function, the code simply has to pass in the type of the fight and the enemy name. (I called it "type" because I figured it determined the strength).

---

Now, you can delete those other 3 functions: all you have to use is this now!

However, there are still a few things I'd like to point out.

---

fdmg1 = int(random.randint(1,7))
edmg1 = int(random.randint(1,6))

This function has it's own scope, so you don't need to worry about other functions using the variables fdmg and edmg and messing up your code. These should be renamed to fdmg and edmg respectively.

---

The function:

`random.randint`

already returns an int, so there is no point in calling int on the return.