Lesson 16.2 – Refactoring the player movement function

Lesson Objectives

At the end of this lesson, you will know…

• How to break a large function into smaller, easier-to-understand functions
• How to move functions from the UI to the “business classes”

In the last lesson, we created a huge function to move a player to a new location. However, it was too big to easily maintain.

Now, we’re going to clean it up and move things around so it is easier to read and understand. This is often called “refactoring”.

Refactoring is a large subject, and there are many techniques you can use to do it. We’ll focus on a couple of the common, simple, techniques that have the biggest benefits. As you continue writing programs, you’ll learn more refactoring techniques.

Creating the functions to handle user input

Step 1: Start Visual Studio Express 2013 for Desktop, and open the solution.

Step 2: Right-click on the SuperAdventure.cs form, in the SuperAdventure project, then select “View Code”.

Step 3: Double-click on the Player class in the Engine project, and replace it with the code from the Player class here (don’t update the SuperAdventure code yet): https://gist.github.com/ScottLilly/6670da749c4ad7e7bff7

Steps for refactoring

Refactoring is just rearranging your code so it is easier to work with. You’re not looking to add any new features, fixing any bugs, or improving the performance.

There’s a lot you can do for refactoring, but we’ll focus on a couple of the most common techniques.

Step 1: Look for duplicated code.

If you have the exact same code in more than one place, then you can usually move that code to its own function. Then, in the places that used to have that code, change it to call the new function.

This is especially important when you might make changes in the future.

When we created the MoveTo() function, we could have put all that code in the four functions that moved the player in each direction. However, if we ever decided to change how the movement logic worked, we’d need to remember to make the change in four places. If we weren’t paying attention, we might only change three of the four. Then, the game would suddenly start acting strangely every time the player moved in the one direction that didn’t have the change added.

We don’t have duplicated code in this function. We have a few places that look close to each other, but no exact duplicates. So, we’ll look for other ways to refactor this function.

Step 2: Look for code that has one distinct purpose, and move it to its own function.

In this huge function, we have lots of code that matches this description.

For example, lines 56 through 78 checks if there is a required item for a location, and (if so), if the player has it in their inventory. We can move this to its own, smaller function.

When we move this section of code, we should think if it might belong in a better place – maybe a different class. We currently have it in the code for our user interface. However, this code is looking at the player’s inventory. So it makes sense to move it to the Player class.

Look at lines 28 through 48 of the new Player class code that you just added.

public bool HasRequiredItemToEnterThisLocation(Location location)
{
    if(location.ItemRequiredToEnter == null)
    {
        // There is no required item for this location, so return "true"
        return true;
    }
    // See if the player has the required item in their inventory
    foreach(InventoryItem ii in Inventory)
    {
        if(ii.Details.ID == location.ItemRequiredToEnter.ID)
        {
            // We found the required item, so return "true"
            return true;
        }
    }
    // We didn't find the required item in their inventory, so return "false"
    return false;
}

In this function, we pass in the location and see if the player can move there – either because there is no required item, or because they have the required item in their inventory.

This new HasRequiredItemToMoveToThisLocation() function is 20 lines long. It does one thing, and is small enough that it’s very easy to understand. If we ever want to change this logic, we’ll be able to do it in this one place.

For example, you might want to change the game to also have a minimum level requirement for a player to enter certain locations. You can go to this function and easily make the change, instead of digging through the 300 line function.

Now that we have the function in the Player class, we can clean up the SuperAdventure class. Replace lines 57 through 78 with this code:

//Does the location have any required items
if(!_player.HasRequiredItemToEnterThisLocation(newLocation))
{
    rtbMessages.Text += "You must have a " + newLocation.ItemRequiredToEnter.Name + " to enter this location." + Environment.NewLine;
    return;
}

We just replaced 20 lines of code with 6, and made this function a little easier to read and understand. That’s what refactoring is all about.

Now we’ll move the code that checks if the player already has a quest, and if they’ve completed it, to the Player class.

Look at the two functions in the Player class at lines 50 through 74:

public bool HasThisQuest(Quest quest)
{
    foreach(PlayerQuest playerQuest in Quests)
    {
        if(playerQuest.Details.ID == quest.ID)
        {
            return true;
        }
    }
    return false;
}
public bool CompletedThisQuest(Quest quest)
{
    foreach(PlayerQuest playerQuest in Quests)
    {
        if(playerQuest.Details.ID == quest.ID)
        {
            return playerQuest.IsCompleted;
        }
    }
    return false;
}

In the SuperAdventure.cs class, replace lines 86 and 87 with these calls to the player object, and delete lines 88 through 100.

bool playerAlreadyHasQuest = _player.HasThisQuest(newLocation.QuestAvailableHere);
bool playerAlreadyCompletedQuest = _player.CompletedThisQuest(newLocation.QuestAvailableHere);

Now, let’s move the code that checks if the player has all the quest completion items in their inventory.

In the Player class, look at lines 76 through 106:

public bool HasAllQuestCompletionItems(Quest quest)
{
    // See if the player has all the items needed to complete the quest here
    foreach(QuestCompletionItem qci in quest.QuestCompletionItems)
    {
        bool foundItemInPlayersInventory = false;
        // Check each item in the player's inventory, to see if they have it, and enough of it
        foreach(InventoryItem ii in Inventory)
        {
            if(ii.Details.ID == qci.Details.ID) // The player has the item in their inventory
            {
                foundItemInPlayersInventory = true;
                if(ii.Quantity < qci.Quantity) // The player does not have enough of this item to complete the quest
                {
                    return false;
                }
            }
        }
        // The player does not have any of this quest completion item in their inventory
        if(!foundItemInPlayersInventory)
        {
            return false;
        }
    }
    // If we got here, then the player must have all the required items, and enough of them, to complete the quest.
    return true;
}

Go back to the SuperAdventure.cs and replace line 96 with this:

bool playerHasAllItemsToCompleteQuest = _player.HasAllQuestCompletionItems(newLocation.QuestAvailableHere);

Then, remove lines 97 through 133.

We can also move the code that removes the quest completion items from the player’s inventory to the Player class.

In the Player class, we have this new function at lines 108 through 122:

public void RemoveQuestCompletionItems(Quest quest)
{
    foreach(QuestCompletionItem qci in quest.QuestCompletionItems)
    {
        foreach(InventoryItem ii in Inventory)
        {
            if(ii.Details.ID == qci.Details.ID)
            {
                // Subtract the quantity from the player's inventory that was needed to complete the quest
                ii.Quantity -= qci.Quantity;
                break;
            }
        }
    }
}

Then go back to SuperAdventure.cs and remove lines 106 through 117, and replace them with this:

_player.RemoveQuestCompletionItems(newLocation.QuestAvailableHere);

We can also move the code that adds the reward item to the player’s inventory into the Player class.

In Player.cs, look at lines 124 through 139:

public void AddItemToInventory(Item itemToAdd)
{
    foreach(InventoryItem ii in Inventory)
    {
        if(ii.Details.ID == itemToAdd.ID)
        {
            // They have the item in their inventory, so increase the quantity by one
            ii.Quantity++;
            return; // We added the item, and are done, so get out of this function
        }
    }
    // They didn't have the item, so add it to their inventory, with a quantity of 1
    Inventory.Add(new InventoryItem(itemToAdd, 1));
}

In SuperAdventure.cs, replace lines 119 through 138 with this:

_player.AddItemToInventory(newLocation.QuestAvailableHere.RewardItem);

In Player.cs, look at the function at lines 141 through 154:

public void MarkQuestCompleted(Quest quest)
{
    // Find the quest in the player's quest list
    foreach(PlayerQuest pq in Quests)
    {
        if(pq.Details.ID == quest.ID)
        {
            // Mark it as completed
            pq.IsCompleted = true;
            return; // We found the quest, and marked it complete, so get out of this function
        }
    }
}

And go back to SuperAdventure.cs. Change lines 122 through 132 to this:

_player.MarkQuestCompleted(newLocation.QuestAvailableHere);

In the rest of the function, we have some code that updates the ComboBoxes and DataGridViews, since the player’s inventory may have changed because of completing a quest.

In the SuperAdventure.cs class, create these new functions:

private void UpdateInventoryListInUI()
{
    dgvInventory.RowHeadersVisible = false;
    dgvInventory.ColumnCount = 2;
    dgvInventory.Columns[0].Name = "Name";
    dgvInventory.Columns[0].Width = 197;
    dgvInventory.Columns[1].Name = "Quantity";
    dgvInventory.Rows.Clear();
    foreach(InventoryItem inventoryItem in _player.Inventory)
    {
        if(inventoryItem.Quantity > 0)
        {
            dgvInventory.Rows.Add(new[] { inventoryItem.Details.Name, inventoryItem.Quantity.ToString() });
        }
    }
}
private void UpdateQuestListInUI()
{
    dgvQuests.RowHeadersVisible = false;
    dgvQuests.ColumnCount = 2;
    dgvQuests.Columns[0].Name = "Name";
    dgvQuests.Columns[0].Width = 197;
    dgvQuests.Columns[1].Name = "Done?";
    dgvQuests.Rows.Clear();
    foreach(PlayerQuest playerQuest in _player.Quests)
    {
        dgvQuests.Rows.Add(new[] { playerQuest.Details.Name, playerQuest.IsCompleted.ToString() });
    }
}
private void UpdateWeaponListInUI()
{
    List<Weapon> weapons = new List<Weapon>();
    foreach(InventoryItem inventoryItem in _player.Inventory)
    {
        if(inventoryItem.Details is Weapon)
        {
            if(inventoryItem.Quantity > 0)
            {
                weapons.Add((Weapon)inventoryItem.Details);
            }
        }
    }
    if(weapons.Count == 0)
    {
        // The player doesn't have any weapons, so hide the weapon combobox and "Use" button
        cboWeapons.Visible = false;
        btnUseWeapon.Visible = false;
    }
    else
    {
        cboWeapons.DataSource = weapons;
        cboWeapons.DisplayMember = "Name";
        cboWeapons.ValueMember = "ID";
        cboWeapons.SelectedIndex = 0;
    }
}
private void UpdatePotionListInUI()
{
    List<HealingPotion> healingPotions = new List<HealingPotion>();
    foreach(InventoryItem inventoryItem in _player.Inventory)
    {
        if(inventoryItem.Details is HealingPotion)
        {
            if(inventoryItem.Quantity > 0)
            {
                healingPotions.Add((HealingPotion)inventoryItem.Details);
            }
        }
    }
    if(healingPotions.Count == 0)
    {
        // The player doesn't have any potions, so hide the potion combobox and "Use" button
        cboPotions.Visible = false;
        btnUsePotion.Visible = false;
    }
    else
    {
        cboPotions.DataSource = healingPotions;
        cboPotions.DisplayMember = "Name";
        cboPotions.ValueMember = "ID";
        cboPotions.SelectedIndex = 0;
    }
}

Then replace lines 183 through 272 with these lines:

// Refresh player's inventory list
UpdateInventoryListInUI();
// Refresh player's quest list
UpdateQuestListInUI();
// Refresh player's weapons combobox
UpdateWeaponListInUI();
// Refresh player's potions combobox
UpdatePotionListInUI();

Is the function simpler now?

Before, the MoveTo() function was over 300 lines long. Now, it’s 140.

It’s still long, but it’s much easier to read (and understand) now.

We’ve also moved a lot of the “logic” code out of the user interface class – which is a good thing. The code in the user interface class should only be used to handle receiving input from the user, and displaying output. It shouldn’t have a lot of logic in it.

Now we have more of the game logic in the Engine project, where it belongs.

We could do more refactoring on this function, but I think this is a good place to stop for now.

If you’re interested in going further, I suggest you look at .Net’s LINQ. You can use it to make the new functions in the Player class even smaller, and more concise. But LINQ is a whole other thing to learn, and I won’t be showing it in these starter tutorials.

Summary

Refactoring doesn’t change what a program does, it only cleans up the existing code, so it’s simpler to understand and work with.

We often do that by finding pieces of a function that can be moved to their own function. Then we have the original function call these new, smaller functions. The smaller functions are easier to understand, since they aren’t buried in a huge function. And the big function is easier to read, since it is shorter and more concise.

NOTE: There were a lot of changes to the code. If you’re not sure that you typed everything correctly, you can paste the code from the link below into the SuperAdventure.cs class.

Source code for this lesson

Source code on GitHub

Source code on Dropbox

Next lesson: Lesson 16.3 – Functions to use weapons and potions

Previous lesson: Lesson 16.1 – Writing the function to move the player

All lessons: Learn C# by Building a Simple RPG Index