2
\$\begingroup\$

Curious on how I can consolidate or generally streamline my code here:

import random

import random

#Rolls a certian number sided dice
def diceRoll(num):

    roll = random.randint(1 , num)

    #print("The roll was: ", roll)

    return roll

############
print("")
############

#Generates a dungeon with a number of rooms based on the dice and roll
def generateDungeon():
    diceRoll(6)

    numberOfRooms = diceRoll(6)

    #print("The roll was: ", roll6)

    if numberOfRooms > 1:
        print("There are ", numberOfRooms, "rooms in this dungeon.")

    else:
        print("There is only 1 room in this dungeon.")

    return numberOfRooms

############
print("")
############

#Defines the size and events of a room the players enter
def randomRoom():
    diceRoll(4)

    numberOfRooms = generateDungeon()

    roomSize = diceRoll(4)

    while numberOfRooms != 0:

        #print("The roll was: ", roll4)

        if roomSize == 1:
            print("You come a small room")
            enemyInRoom()
            chanceOfChest()


        elif roomSize == 2:
            print("You to a normal sized room")
            enemyInRoom()
            chanceOfChest()

        else:

            if roomSize == 3 or roomSize == 4:

                diceRoll(10)

                largeRoom = diceRoll(10)

                if largeRoom <= 5:
                    print("You come to normal sized room")
                    enemyInRoom()
                    chanceOfChest()
                else:
                    print("You come to a large room")
                    enemyInRoom()
                    chanceOfChest()

        numberOfRooms -= 1


############
print("")
############


#Defines the chance of finding a chest in a room
def chanceOfChest():
    diceRoll(20)

    chestChance = diceRoll(20)

    #print("The roll was: ", chestChance)

    if chestChance >= 17:
        findAChest()

    elif chestChance == 1:
        print("You find a chest and decide to look inside... It's a mimic!")
        print("")
        print("After battle, you continue on")

    else:
        print("There appears to be nothing of value in this room.")


############
print("")
############


#Defines the chance of obtaining certain items in a chest
def findAChest():
    diceRoll(20)

    roll = diceRoll(20)
    gold = diceRoll(20) * .75
    platinum = diceRoll(20) * .5
    emperium = diceRoll(20) * .1

    foundChest = input("You've found a chest! Would you like to open it? ")

    if foundChest == "yes" or foundChest == "Yes":

        #print("The roll was: ", roll20)

        if roll == 1:
            print("The chest is a mimic! Prepare for battle!")


        elif roll < 15:
            print("Sadly, the chest is empty. You move on from the chest.")


        elif roll < 18:

            print("You find ", format(gold, "3.0f"), "Gold in the chest!")

            lookAgain = input("Would you like to look again? ")

            if lookAgain == "yes" or foundChest == "yes":

                diceRoll(20)

                if roll == 20:

                    #print("The roll was: ", roll20)

                    print("You find ", format(gold, "3.0f"), " more Gold in the chest!")

                else:
                    print("You don't see anything else and move on from the chest.")

        elif roll < 19:

            print("You find ", format(platinum, "3.0f"), "Platinum in the chest!")

            lookAgain = input("Would you like to look again? ")

            if lookAgain == "yes" or foundChest == "yes":

                diceRoll(20)

                if roll == 20:

                #print("The roll was: ", roll20)

                    print("You find ", format(gold, "3.0f"), " more Gold in the chest!")

                else:
                    print("You don't see anything else and move on from the chest.")



        elif roll < 20:

            print("You find ", format(emperium, "3.0f"), "Emperium in the chest!")


            lookAgain = input("Would you like to look again? ")

            if lookAgain == "yes" or foundChest == "yes":

                diceRoll(20)

                if roll == 20:

                #print("The roll was: ", roll20)

                    print("You find ", format(gold, "3.0f"), " more Gold in the chest!")

                else:
                    print("You don't see anything else and move on from the chest.")

        else:

            diceRoll(20)

            if roll >= 17:

                print("You find a magical item in the chest!")
    print("")


    if foundChest == "No" or foundChest == "no":
        print("You move on and leave the chest behind.")

    print("")

#################
print("")
#################

#Determines if enemies are in a room and how many
def enemyInRoom():
    diceRoll(20)

    enemyAppears = diceRoll(20)

    diceRoll(6)

    numberOfEnemies = diceRoll(6)

    if enemyAppears == 1:
        print("You walk into the room and it appears to be empty.", end="")
        print("Then you hear the door close behind you. ", end="")
        print("You turn to find", numberOfEnemies + 2 , "enemies waiting in ambush!")
        print("")
        print("After defeating the enemies the room appears to be safe.")
        print("")

        searchRoom = input("Do you want to search the room? ")

        if searchRoom == "yes" or searchRoom == "Yes":
            chanceOfChest

        else:
            print("You decide to move on from this room")


    elif enemyAppears < 15:
        print("You open the door to find", numberOfEnemies, "enemies!")
        print("")
        print("After defeating the enemies the room appears to be safe.")
        print("")

        searchRoom = input("Do you want to search the room? ")

        if searchRoom == "yes" or searchRoom == "Yes":
            chanceOfChest

        else:
            print("You decide to move on from this room")

    else:
        print("This room appears to be safe.")

        searchRoom = input("Do you want to search the room? ")

        if searchRoom == "yes" or searchRoom == "Yes":
            chanceOfChest

        else:
            print("You decide to move on from this room")

#Runs program and has each section commented out for troubleshooting
def dungeon():
#diceRoll(20)
#findAChest()
#generateDungeon()
    randomRoom()
#chanceOfChest()
#enemyInRoom()


dungeon()

#Commented out print statements are for troubleshooting and confirming
#that the rolls and results are giving correct outputs

As of posting this it appears to be working in all of the situations that I've been able to test. If you see any errors by all means let me know.

\$\endgroup\$

closed as unclear what you're asking by Graipher, esote, Toby Speight, Mast, Edward Apr 18 at 13:23

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    \$\begingroup\$ Welcome to Code Review! Please describe what you want to accomplish with your program. Be as detailed as possible. After that, also update your title accordingly. See Asking your question and Titling your question here. \$\endgroup\$ – AlexV Apr 13 at 7:56
2
\$\begingroup\$

There are indeed a few aspects of your code that need work. The following review will use some techniques introduced in Python 3. If you're not yet using Python 3, you definitely should, especially if you do not already have a large code base that only works in Python 2. Python 2 will only be actively maintained until 2020.

Style and Structure

First, the obligatory note about Python's official Style Guide, often just called PEP8 fort short. You definitely should read and probably also follow it since it is quite a neat collection of general best practices when it comes to code style and structure in Python. I will walk through some aspects in more detail in the following.

The first point I would like to talk about in more detail is documentation. You have shown the will to document your functions. The Style Guide prescribes the preferred way to document functions and the like in the section Documentation Strings. Using """...""" to document your code has not only the benefit of being style guide compliant, but also helps Python's built-in help function and almost all Python IDEs to pick up this documentation. It would be used like this:

def diceRoll(num):
    """Rolls a certain number sided dice"""
    # your other code here

While we are at names, the official recommendation for variable and function names is snake_case. Most Python programmers tend to follow this recommendation, but as long as you use a consistent style I would accept alternatives. The code piece above could simply be rewritten to follow the style guide as follows:

def dice_roll(num):
    """Rolls a certain number sided dice"""
    # your other code here

Another thing to note about names that is independent of the Style Guide or the language you use, are the names themselves. It might not be an issue with simple code like yours, but later on if the code gets more complex, you will start to hate yourself if the variable and function names hint in one direction and do something else. I would consider chanceOfChest() a good (or bad?) example for this. chanceOfChest() sounds like a function that would return a probability of how likely it is that a player can find a chest. Instead of this, there is a whole event chain scripted inside that decides a) if the player finds a chest at all and b) generates the content of the chest using another function. I think something like generate_chest_event() or the like yould be more appropriate here. That applys to a lot of your other functions as well. The example from above is also quite interesting in that context since it is one of a few functions that break the "do something" name scheme. To be more in line with the rest of the code I would make another edit there:

def roll_dice(n_sides):
    """Rolls a certain number sided dice"""
    # your other code here

After looking at the code on a zoomed in manner for quite a while now, I would like to take a step back with you and look at the greater structure of it. For the time being, think about structure more as the (superficial) strucural appearance of the code, not the algorithmic structure. You seem to understand that blank lines can greatly help to visually group code blocks that belong together. Unfortunately, you're not doing a great job when it comes to applying that principal to your code. Let's look at an example from your question:

import random

#Rolls a certian number sided dice
def diceRoll(num):

    roll = random.randint(1 , num)

    #print("The roll was: ", roll)

    return roll

############
print("")
############

#Generates a dungeon with a number of rooms based on the dice and roll
def generateDungeon():
    diceRoll(6)

    numberOfRooms = diceRoll(6)

    #print("The roll was: ", roll6)

    if numberOfRooms > 1:
        print("There are ", numberOfRooms, "rooms in this dungeon.")

    else:
        print("There is only 1 room in this dungeon.")

    return numberOfRooms

Blank lines literally everywhere. Again, PEP8 has guidelines on how to use blank lines to actually support the structure of your code, and not distort it. Applying these guidelines to that snippet together with the aspects mentioned above would lead to code that looks more like the following snippet:

import random


def roll_dice(n_sides):
    """Rolls a certain number sided dice"""
    roll = random.randint(1, n_sides)
    #print(f"The roll was {roll}")
    return roll


def choose_random_dungeon_size():
    """Generates a dungeon with a number of rooms based on the dice and roll"""
    number_of_rooms = roll_dice(6)
    if number_of_rooms > 1:
        print(f"There are {number_of_rooms} rooms in this dungeon.")
    else:
        print("There is only 1 room in this dungeon.")
    return number_of_rooms

Quite a few things1 have happened while getting there, so let me talk you through it. I massively cut down on blank lines within the functions, and added a few where they are more appropriate. Here "appropriate" means that there are two blank lines between larger pieces of code that work on their own, such as the import section as well as between each of the two functions. I also got rid of the loose pieces like the dice roll whose value was never assigned to a variable and thus had no effect and one of those weird print statements that are found quite often in your code. There is no need for those prints since all they will buy you are a number of blank lines before your code is printing its first output to the console. It neither helps you, a revier nor the Python interpreter to make any more sense of the written code.

Since running has just come up, let me introduce you to another best practice when writing Python code that is supposed to be run as script. There is general distinction between parts of code you would write to use/import as a library such as random (or countless other modules/libraries shipped with Python be default) and parts to be used as scripts that use those library functions to actually "do something". While looking at code written by more experienced Python programmers you will often find a code block saying

if __name__ == "__main__":
    main()

often quite at the bottom or the top of a script. That is the Python way of checking if the current source code is run as a script (in which case __name__ == "__main__" is True) versus someone uses it as a library. In that case the main function would be run. If you were to import the file from another Python file you have written, __name__ would be different and main would not be executed, but you could use all the other functions (theoretically also main) at your discretion. See also the Python documentation and this SO post for further reading.

Incorporating all or parts of these changes in your code will greatly improve the code's readability and maintainabilty.


Since this answer is alreay quite long, I will stop here and leave comments on the code itself to other members of the community or maybe even future-me.


1I also introduced so called f-strings, which is as of Python 3.6 the recommended way of producing formatted strings. There is a nice blog post here presenting and comparing all the string formatting techniques available in Python and how to properly use them.

\$\endgroup\$

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