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.

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:

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:

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


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:

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

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:

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


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:

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


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

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


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:

Then replace lines 183 through 272 with these lines:


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.



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

50 thoughts on “Lesson 16.2 – Refactoring the player movement function

  1. You should show us what code to replace, not tell us what line. Our code might not look 100% identical to yours, so telling people what lines to replace/delete isn’t amazingly helpful.

    1. Totally agree with Timothy Harrelson’s comment. Show us what code to replace. In my case, I was making my own comments in the code as an aid to memory for everything I’ve learned in the lessons up until this point, so due to those comments, my code was about 20 lines out of sync with yours. At the end of the day, I simply copy-pasted your code over my own (hopefully I shouldn’t need the comments anymore, after all that’s the point of trying to learn) but as Timothy notes, we all have slightly different ways of doing things, such that absolutes like line numbers only work for those who don’t deviate even slightly.

      Otherwise, it’s been a great tutorial and I’m very happy you went to all this effort! After getting halfway thought Herbert Schildt’s “C# 4.0: The Complete Reference”, I found your tutorial to be a welcome breath of fresh air, bringing simplicity to many concepts in a fun and practical context. Thanks!

      1. Thanks. This was a tough lesson to figure out how to write. Every way I could think of had one problem or another. Although, it does show why refactoring large functions is important (since they are difficult to work with). I may do a separate guide on refactoring in the future.

        1. I agree with the commenters above, and I think there’s a pretty simple solution. Since you have annotated the SuperAdventure code very well, if you just refer to the annotation (e.g. “(below // See if the player already has the quest, and if they’ve completed it)”)instead of or along with the line number, that should really help.

  2. I’m kind of stuck here. If I run the Game and try to walk right in the beginning to the North by pressing the North Button, nothing happens. VS doesn’t throw anny Errors or something. Somebody able to help?

    1. The first thing to check is the SuperAdventure.cs class. Ensure this function exists:

      private void btnNorth_Click(object sender, EventArgs e)

      If the function exists, go to the design mode of SuperAdventure.cs and click once on the “North” button. In the “Properties” section (lower-right corner), click on the lightning symbol (to see the events for the button) and see if the “Click” event has a value of “btnNorth_Click”. That’s where the button’s click event connects to the function.

      Click event for the 'North' button, in the 'Properties' section of Visual Studio

      If the click event is empty, double-click on SuperAdventure.Designer.cs (in Solution Explorer) and scroll until you find the btnNorth section (shown in the image below). Then add the missing line:

      Code in the code-behind page for SuperAdventure.cs that connects the 'North' click button to the function to run when it is clicked.

      this.btnNorth.Click += new System.EventHandler(this.btnNorth_Click);

      Please let me know if that does not fix the problem.

      1. The Buttons didn’t had the Click Event saved. After adding the events to the Buttons everything works fine now!!
        btw. it’s a great Tutorial!! Thank you for that.

  3. Im a little lost on what code Im replacing.

    I am suppose to replace line 57-78 which for me is



    return false;


    public bool CompletedThisQuest(Quest quest)


    foreach(PlayerQuest playerQuest in Quests)


    if(playerQuest.Details.ID == quest.ID)


    return playerQuest.IsCompleted;



    return false;


    public bool HasAllQuestCompletionItems(Quest quest)


    // See if the player has all the items needed to complete the quest here

    which makes me a bit confused. could you please advice me on which code Im replacing?

  4. Hi Scott,


    In this lesson I replaced both my forms code as well as the player class’ code with the code you put on your github repository at the end of this lesson. Later on when running the game I noticed my comboboxes (to select weapons ) didnt stay visible on the UI. I figured it out because the forms constructor used to have a line that adds a rusty sword to the inventory of the player. In the new version this line was removed. So whenever the moveto method was called and the location would have a monster, it would instantiate a monster, then put my combobox and respective buttons Visible property to true, then run the update UI methods which would find no weapons in inventory, thus immediately hiding them again.

    I fixed it by re-adding this to the forms constructor:

    _player.Inventory.Add(new InventoryItem(World.ItemByID(World.ITEM_ID_RUSTY_SWORD), 1));


    Just pointing this out in case people have problems with it.


    Kind regards,



    1. Thanks. I’ll take a look at this and tried to get it resolved. At one point, I moved that line from SuperAdventure.cs into the Player constructor. I might have missed something when I made that change.

  5. Hello Scott,


    first of all. Thank you very much for this great tutorial. I’ve never seen something that shows the using of objects so clear like here!

    I’m sure you can help me with this issue I get when I push the button “North” the first time:

    In my Player class I get a NullReferenceException on this line:

    if (location.ItemRequiredToEnter == null)


    in the function:

    public bool HasRequiredItemToEnterThisLocation(Location location)




    1. It might be coming from the World.PopulateLocations function. Can you double-check that function, to be sure all the LocationToNorth, LocationToEast, etc. values are set?

      If they look correct, can you upload your solution (all the files, including the one in the sub-directories) to Dropbox or GitHub, so I can look at it? There are a few places I might need to check, to find exactly where the problem is happening.

      1. Hi Scott,

        I double checked the World class. As far as I Can see the locationToNorth is distributed correctly.
        The alarm happens when the compiler is coming to the if statement where it looks if the Item to enter is Null.
        I’ve looked about that process with some breakpoints and debug it.
        This Information I got:
        locationToNorth=Engine.location –> Thats fine for me. I can enter the area to the north.
        ItemRequiredToEnter= null –> This is why the NullReferenceException happen (I guess) But I don’t understand why it happen because we set this Variable to Null(because we don’t need anything to enter) and we just ask with the if statement “if its Null then return True”

        I try to setup my Dropbox later on when I get a bit more time.

        Thanks, CodeB

        1. Hi there,

          I found my fault:
          The problem was in the SuperAdventure.Designer.cs file.
          Here I had this code line:
          this.btnNorth.Click += new System.EventHandler(this.btnWest_Click);

          it must be:
          this.btnNorth.Click += new System.EventHandler(this.btnNorth_Click);

          As well I had some other very simillar problems with this buttonevents.
          This all was happend because I forgot to doubleclick some of the buttons at the very beginning of the tutorial.

          Now it works fine and I can go on.


          1. I am getting the exact same error, but your fix below is not the same for me.
            This is driving me crazy.

  6. Hello,

    i did a huge mistake and dbl-clicked on a Label somewhere, at a time i didnt noticed and now i got this Error:

    ‘SuperAdventure’ does not contain a Definition for ‘SuperAdventure_Load’ and no Extension method ‘SuperAdventure’ accepting a first Argument of type ‘SuperAdventure’´could be found (are you missing a using directive or an assembly reference?)

    On the SuperAdventure.cs[Design]
    i recieve the error “The designer cannot process unknown Name ‘SuperAdventure_Load” at Line 287. The Code within the method ‘initializeComponent’ is generated by the designer and should not be manually modified. Please remove any changes and try opening the designer again.

    Would be great if you could help me again with my error.

    Best regards

    1. To fix that problem, open SuperAdventure.Designer.cs

      Solution Explorer, with SuperAdventure expanded to show SuperAdventure.Designer.cs

      Look for line 287 (which will have the line for the SuperAdventure_Load eventhandler). You can either delete that line, or put two forward slashes in front of it “//”, to comment it out.

      You can learn more about how the eventhandlers work in Lesson 21.3

      Please tell me if that does not fix the problem.

  7. Hello,

    First of, thank you for this tutorial. I’m learning C# at school but I really like the way you teach us.

    I come here because I have a problem with my game. I’ve made the .exe file, I can use the UI but the game doesn’t work properly. I can’t manage to find a solution, that’s why I’m asking you to help me if you don’t mind …

    Here is a screenshot of the finale program output (once compiled):


    Here is my whole project (in a .rar file):


    Thank you in advance,



    1. Hello Jean,

      The problem is that the movement and combat buttons are not connected to the function in SuperAdventure.cs. They are missing their “eventhandlers”. You can fix this by reading Lesson 21.3, and following step 2 for the buttons. Or, you can open the SuperAdventure form in Design mode. Click on each button once. After you click on it, look at the “Properties” section of Visual Studio (in the lower-right corner). Click on the lightning bolt, to see the events for the button. Find the “Click” event, and click on the area to the right of it. That will show you the available functions. Select the correct function for each button. That will create the “eventhandler”, and should fix your program.

      Please tell me if that does not work, or if you have more questions.

      1. Hi. I checked to see if the btnNorth_Click, all had the right event handler. It still does not work. Love your tutorial btw.

        -Rasmus from Denmark

  8. I keep getting this error when running the application:
    public bool HasRequiredItemToEnterThisLocation(Location location)
    if (location.ItemRequiredToEnter != null)
    // There is no required item for this location, so return “true”
    return true;
    “An unhandled exception of type ‘System.NullReferenceException’ occurred in Engine.dll”

    I have looked at solution for a prior commenter, but that is not the solution for me. I have checked every reference and everything has been coded correctly. I even went back, copied and pasted the code you supplied, and I still get this error.

    Please help!!!

  9. Hello Scott. Thanks for this GREAT guide. I’ve spent whole September with it and I’m loving it 😉
    Got one questions though:
    My weapons list displayed in cbWeapons has 3 weapons in it.
    Whenever I change location, selected item goes back to the first one on the list. This can be annoying, especially when player wants to keep using different weapon (e.g. with index 3) all the time.

    This is caused by: cbWeapons.SelectedIndex = 0;

    Is there any way to save item’s index that was selected and keep it when player moves into new location?

  10. Hello. I get this message when i pasted the last code in.

    “Severity Code Description Project File Line Suppression State
    Error CS0106 The modifier ‘private’ is not valid for this item SuperAdventure C:\Users\ander\Documents\C# projekter\SuperAdventure\WindowsFormsApp1\SuperAdventure.cs 193 Active”

    This is on all of the private methods i pasted in from the last box we had to paste in.

    1. That’s a strange error to see here. The first thing I can think of that might cause that error is if the code was pasted outside the curly braces “{}” for the class.

      If you can’t see if that is the problem, can you upload all the files of your solution (including the directories under it, and the files in those directories) to GitHub or Dropbox, so I can look at it?

  11. Hi Scott !
    Now with realisation more things i’ve got some silly questions that bother me…
    1) ” (ii.Details.ID ” Why do we write “Details”
    “if(playerQuest.Details.ID == quest.ID)” (in this case for example why can’t we write just playerQuest.ID
    By the way playerQuest has no a property ID) ?
    When i think about this i get that ii has a property Details, while Details has the property ID, but i was serching for that propery “Details” in Locations ( Because we used datatype Locations to pass in ) and there was nothing. Even wheni found some proprorties like “Details” there were nothing about theirs proporties like ID.
    I guess, that it’s because the “Details” proportie is in fact a calss like “Math” that are included by default in .NET

    2) “public bool CompletedThisQuest(Quest quest)” – as i’m getting it it is a method called CompletedThisQuest, where we get bool and put Datatype Quest, but why there is a another word “quest”, because this is not a Datatype, it’s an object.

    Sorry for my stupidity, how wrong am i ?
    And a question about your lessons, should i firsly try to write code by myself and only then look at yours? Because till this time i’ve been just bluntly coppying your code and then just reading your descriptions.

    1. Don’t answer on second question, i looked again on some points in your couse and i understood that this is just a initilisation of a new variable that we’re going to use.
      But while thinking about that i got anoter one… What’s the accebility by default, if we don’t write public or private?
      Really thank your for your course, i don’t know where else could i finily start envolving into programming.

    2. 1) You are thinking correctly. “ii” has a “Details” property, and the “Details” property is an object (an Item object) has an “ID” property. But, the “ii” variable is an InventoryItem object, not a Location object.

      I see in your other comment that you understand question 2 now.

      Everyone learns differently. But, you might try reading the descriptions first, then looking at the code. Try to understand how the code is working before copying it into your program. When you look at a function, try to imagine it running. The parameter variables going into it, then being processed. Think how the function would run differently with different values (maybe some values go through an “if”, and other values go through an “else”).

      You might also want to try using the debugger. It lets you watch the code (and variables) when the program is running. I have a lesson on how to use the debugger here.

  12. Hi scott i keep on getting errors in my program

    for this line of code:
    rtbMessages.Text += “You must have a ” + newLocation.ItemRequiredToEnter.Name + ” to enter this location.” + Environment.NewLine;
    i am getting an error saying:

    Error CS1061 ‘object’ does not contain a definition for ‘ID’ and no extension method ‘ID’ accepting a first argument of type ‘object’ could be found (are you missing a using directive or an assembly reference?)

    i am also getting this error for this line of code:
    if (ii.Details.ID == itemToAdd.ID)
    Error CS1061 ‘InventoryItem’ does not contain a definition for ‘ID’ and no extension method ‘ID’ accepting a first argument of type ‘InventoryItem’ could be found (are you missing a using directive or an assembly reference?)

    And finally i am getting an error on this line of code:
    if (ii.Details.ID == location.ItemRequiredToEnter.ID)
    i get this error:
    Severity Code Description Project File Line Suppression State
    Error CS1061 ‘object’ does not contain a definition for ‘ID’ and no extension method ‘ID’ accepting a first argument of type ‘object’ could be found (are you missing a using directive or an assembly reference?

    I dont understand what i am supposed to do to fix these.

    1. Those errors are saying that the program is looking for an ID property on an object, but cannot find it.

      Check the Item class, and make sure it has:
      public int ID { get; set; }

      The problem also might come from the InventoryItem class. Make sure it matches the code at: https://gist.github.com/ScottLilly/161a5812ae4843563d6b#file-inventoryitem-cs

      Some common problems are: not including the “public” or having different upper/lower-case on the property name.

      If that does not help you fix the errors, can you upload your solution (including the directories under it, and all the files in those directories) to GitHub or Dropbox, so I can look at it?

    1. The ItemRequiredToEnter property in the Location class is currently:
      public object ItemRequiredToEnter { get; set; }
      but needs to be:
      public Items ItemRequiredToEnter { get; set; } (since you named your Item class “Items”)

      In the Player class, the AddItemToInventory function needs to accept the parameter as “Items”, instead of “InventoryItem”. So, line 124 should be:
      public void AddItemToInventory(Items itemToAdd)

      You will also need to delete the second AddItemToInventory function on lines 155-159.

      Let me know if that doesn’t fix the errors.

  13. Hello Scott, thanks for the tutorials!

    I have been following this lesson to the exact point but end up with still over 200 lines of code and more then enough errors, which I can’t seem to solve. My code before this lesson was exactly the same as it should be but for whatever reason when removing lines, then copy and pasting the new ones in, I just get more errors and lines that look a mess.

    I visual example from yourself would have been really helpful.

  14. Hello! Loving the tutorial!
    I am having one error on line 315 in SuperAdventure.Designer.cs

    the code on that line: this.Load += new System.EventHandler(this.superAdventure_Load);

    give this error:
    Severity Code Description Project File Line Suppression State
    Error CS1061 ‘SuperAdventure’ does not contain a definition for ‘superAdventure_Load’ and no accessible extension method ‘superAdventure_Load’ accepting a first argument of type ‘SuperAdventure’ could be found (are you missing a using directive or an assembly reference?)

    any advice would be greatly appreciated,


    1. Hi Ben,

      It sounds like you might have accidentally double-clicked on the SuperAdventure form while in the Design mode screen. If someone does that, Visual Studio will create a new function and an “eventhandler” for that function. You can read more about eventhandlers in Lesson 21.3.

      The solution is to edit the SuperAdventure.Designer.cs file and delete line 315 (the one with the “this.Load += new System.EventHandler(this.superAdventure_Load);”)

      Let me know if you try this and it does not fix the problem.

  15. I was just thinking about how much time must have gone into supporting this tutorial over the course of several years. Thank you for that, and for creating it in the first place. I have been teaching myself c# at nights and this tutorial has been a great resource to work through and try and wrap my head around everything.

    My 1 big question that I just can’t quite wrap my head around after this lesson is how you set the datatype to ‘bool’ in the some of these functions – where you check whether a player has a required item (for instance). I see that the returned value is going to be a true/false, but the value being passed in from the SuperAdventure.cs class is not… so in:

    public bool HasRequiredItemToEnterThisLocation(Location location)

    you are basically passing in a ‘Location’ object, where ‘location’ represents the location the player just tried to move to. Everything that happens in that function makes sense, but I don’t understand setting the datatype as bool when it first holds an object?

    I’m sure this is just a ‘how I’m thinking about it’ issue, but I can’t make sense of it.



    1. You’re welcome, Michael!

      When we add a property – like “public string Name { get; set; }” – the dataype defines what value we can store in the property.

      But, when we create a function, the datatype in front of the function declares the datatype the function is going to return – not what is going into it. If the function is just going to do some work, and does not need to return anything, it would be a “void” function.

      So, if we were going to have a function that baked a cake (to give a real-world example), it could be defined like “public Cake BakeCake(int amountOfFlour, int amountOfSugar, int numberOfEggs)”. The “Cake” datatype is what the function will return, the “int” parameters are the ingredients the function needs to create that Cake.

      In the HasRequiredItemToEnterThisLocation function, we want it to return a boolean answer/value – can the player enter the location or not? That’s why we have “public bool HasRequiredItemToEnterThisLocation”. The Location parameter is just the “input” the function needs to create its “output”. Inside the function, the datatype of the object in our “return” statetement must match the datatype declared for the function, since that is the “Cake” the function is returning.

      Does that make it clearer?

Leave a Reply

Your email address will not be published. Required fields are marked *