Question about Encapsulation (OOP)

Because it allows you to control exactly how a variable is used.

I had a practical example of this when I developed my iPhone App. Back then the place I was working for wanted to support the iPhone 3GS/4, iPod Touch 3rd/4th gen, and iPad which were the most recent devices. They also wanted the iPad to support native resolution textures (overkill in my opinion, but they were signing my checks).

This means I had two sets of textures, low and high res for the 480x320, 960x640, and 1024x768 resolutions the devices used.

I had an OpenGL bitmap font rendering class that had a scale attribute that controls how much the font was stretched from the original. What I forgot to account for when coding everything (I pretty much coded the entire thing for 3rd gen iPod touch because that was our low end) is that the scale calculations would be slightly different for the retina and iPad displays.

So all I did was add a default scale attribute to my bitmap font class. Took me all of 5 minutes to code and test.

Had I just kept the variable public, I would have to ctrl replace my way through the code or do some grep voodoo.

As a general rule of programming you always want to keep your modular and loosely packed so you can easily adjust the pieces you want to without messing up the rest of the code. Encapsulating your variable with accessors and mutators allows you to very easily control and adjust how the variable/object is used.

But what if the variable/field does not need and specific range/requirements, if this is the case should I just declare it as public?

But what if the variable/field does not need and specific range/requirements, if this is the case should I just declare it as public?

It's still handy to be able narrow down where a variable is being accessed down to 6 lines of code from get/set methods. Often times when deubgging a realtime game I'll get weird values for an object variable. Since the only way to get to it is through the accessor method, I can very easily put one conditional breakpoint in the get function as opposed to having to find every occurrence of the variable being accessed and putting break points there.

the ingame class creates instances of -has- relationships (other classes), the thing is though I want these -has- relationship classes (i.e. World.cs, Downloader.cs, Uploader.cs) to have the ability to talk back to the main ingame.cs class and any other -has- relationships within the main ingame.cs

Having two classes that both have knowledge of each other is a sign of an unrefined design (aka a "code smell"). Having a whole bag of classes that all need knowledge of each other, with no signs of layering is a definite sign of an unrefined design.
The fact that encapsulation is a hindrance to this design is just a symptom of the fact that the design isn't following proper OO ideals to begin with.

The point of encapsulation and other OO guidelines is to reduce the area within your code-base that a line of code can have a direct effect on. If you know there's a bug with some part of the code, or you need to modify some part of your code, then a proper design will ensure that when making your changes you've only got to look at and understand a small section of the code, and that after making the change, no other sections of the code will be unintentionally affected by that change.

On the other hand, when everything is public, and every class has knowledge of every other class, it's impossible to perform such reasoning. Any member of any class might possibly be used by any other class, so before making changes to that member, you've first go to read/understand the entire code-base. This isn't a problem on small 1-person projects, but is a nightmare on large projects.

[quote name='Xanather' timestamp='1344754074' post='4968629']
the ingame class creates instances of -has- relationships (other classes), the thing is though I want these -has- relationship classes (i.e. World.cs, Downloader.cs, Uploader.cs) to have the ability to talk back to the main ingame.cs class and any other -has- relationships within the main ingame.cs

Having two classes that both have knowledge of each other is a sign of an unrefined design (aka a "code smell"). Having a whole bag of classes that all need knowledge of each other, with no signs of layering is a definite sign of an unrefined design.
The fact that encapsulation is a hindrance to this design is just a symptom of the fact that the design isn't following proper OO ideals to begin with.

The point of encapsulation and other OO guidelines is to reduce the area within your code-base that a line of code can have a direct effect on. If you know there's a bug with some part of the code, or you need to modify some part of your code, then a proper design will ensure that when making your changes you've only got to look at and understand a small section of the code, and that after making the change, no other sections of the code will be unintentionally affected by that change.

On the other hand, when everything is public, and every class has knowledge of every other class, it's impossible to perform such reasoning. Any member of any class might possibly be used by any other class, so before making changes to that member, you've first go to read/understand the entire code-base. This isn't a problem on small 1-person projects, but is a nightmare on large projects.
[/quote]
Hodgman you are probably completely right, I feel like I'm doing something wrong. Your saying have all main functionality of the game in one class? I should probably look at real examples more. Right now i have a Uploader.cs class Downloader.cs class, Listener.cs class, Connection.cs class, Disconnector.cs class, Messager.cs class, World.cs class, etc... etc... Am I splitting things up too much/far?

Could anyone be able to describe the best way to structure the classes and relations for a game, or any program in that case.

[quote name='Xanather' timestamp='1344762528' post='4968651']
But what if the variable/field does not need and specific range/requirements, if this is the case should I just declare it as public?

It's still handy to be able narrow down where a variable is being accessed down to 6 lines of code from get/set methods. Often times when deubgging a realtime game I'll get weird values for an object variable. Since the only way to get to it is through the accessor method, I can very easily put one conditional breakpoint in the get function as opposed to having to find every occurrence of the variable being accessed and putting break points there.
[/quote]

That does make sense.

Your saying have all main functionality of the game in one class?

Not at all. Each class should have a single responsibility only, which usually means having a lot of different classes.

Every instance of a class you create is always in some sort of state, this state can either be a valid state or an invalid state.
When a class state is valid you have a guarantee that the class works as you intended it to, but you'll need a mechanism to determine what state your class is in.

This is where the concept of the invariant comes into play. The invariant of a class is a set of 'rules' which define the limits in which said class can operate properly.
An invariant will include a description of all the valid values for your class members (ie. values for which your class will show properly defined behaviour), and you can enforce your class to correctly follow this invariant by properly designing your class methods which operate on its data.
Even if you don't explicitly write down a class invariant for yourself, each class still has one since there will always be a situation in which a class can show undefined or unintended behaviour.

Now imagine a class which exposes all of its members publicly.
Every other object holding a reference to an instance of this class is able to directly alter its state, there is no intermediate layer which can make sure that your class instance conforms with its class invariant.
Unless your class can never be in an invalid state (which is pretty much impossible) you'll never be able to guarantee or prove that it is working as intended. It could very well be that some completely unrelated object altered the state of your class instance so that it doesn't conform to its invariant anymore, which can result in undefined behaviour occuring, which is something you absolutely do not want.
Not being able to prove that your class is working correctly also sends you straight into debugging hell, because state alterations are no longer confined to the scope of your class and virtually any part of your program could break its functionality.


Bottom line: The only public members should be members which cannot invalidate your class state, which means that they are safe to use in any part of your program.

[quote name='Xanather' timestamp='1344768457' post='4968676']Your saying have all main functionality of the game in one class?

Not at all. Each class should have a single responsibility only, which usually means having a lot of different classes.
[/quote]
Makes sense, what if you have a Player.cs class and a Spell.cs class (where the spell inflicts damage on the player). How would you implement this that follows OOP principles, because obviously the Spell class will need to communicate with the Player class.

[quote name='Hodgman' timestamp='1344769551' post='4968681']
[quote name='Xanather' timestamp='1344768457' post='4968676']Your saying have all main functionality of the game in one class?

Not at all. Each class should have a single responsibility only, which usually means having a lot of different classes.
[/quote]
Makes sense, what if you have a Player.cs class and a Spell.cs class (where the spell inflicts damage on the player). How would you implement this that follows OOP principles, because obviously the Spell class will need to communicate with the Player class.
[/quote]

There's a difference between objects interacting with eachother and each class having only one responsibility.

You're probably looking for something like this:
[source lang="csharp"]//Pseudocode!

// The character class is a base class for all characters in your game, be it NPCs or players
class Character
{
/* Character specific code here.
This could include changing hit points, adding gold, etc. */
}

// Our hypothetical player class overrides the character class
class Player : Character
{
/* Player specific code here */
}

// Our spell class, we'll make this an interface since you'll probably be implementing a lot of different spells
interface Spell
{
// This is the actual code which will execute the spell and affect a character
void executeSpell(Character character);
}[/source]

In your executeSpell method in your Spell class you can now change the properties of your character instance by using the methods the Character class exposes. Since your Player class has the Character class as its base you are perfectly able to pass in your player object to the executeSpell method.

The Character and Player classes are still completely responsible for managing their own state, and so is the Spell class.


EDIT:

To expand some more on my previous post, let's look at an example. Have a look at this class:
[source lang="csharp"]
class Character
{
/* I'm just adding the method declarations here, we'll see their implementations in a bit */

// Changes the amount of gold the character has
public void changeGold(int amount);

// Changes the character's hit points
public void changeHitPoints(int amount);

// These are our class members
private int m_gold;
private int m_hitPoints;
}[/source]

We're using regular old 32 bit integers here for our class members for simplicity and to make my point a bit clearer.
Let's say we decide that a character can have hitpoints within a range of [0-100] and gold within a range of [0-9999].
These limitations basically set up the class invariant for the Character class:

Invariant for the Character class:

• Member m_gold must be in a range of [0-9999] at all times
  
• Member m_hitPoints must be in a range of [0-100] at all times
  
  [/quote]
  
  In our methods we now make sure that this invariant is adhered to at all times:
  
  [source lang="csharp"]public void changeGold(int amount)
  {
  if (m_gold + amount > 9999)
  {
  // It's up to your design what you want to do in this case, here we'll just max out the character's gold
  m_gold = 9999;
  }
  else if (m_gold + amount < 0)
  {
  // We'll set the amount of gold to 0, since we can't have negative gold
  m_gold = 0;
  }
  else
  {
  m_gold += amount;
  }
  }
  
  // We apply the same principles to the hit points
  public void changeHitPoints(int amount)
  {
  if (m_hitPoints + amount > 100)
  {
  m_hitPoints = 100;
  }
  else if (m_hitPoints + amount < 0)
  {
  m_hitPoints = 0;
  }
  else
  {
  m_hitPoints += amount;
  }
  }[/source]
  Note: In C# you could also use class properties instead of mutator methods like we did here.
  
  
  We are now sure of the fact that our class state will remain valid when using these methods.
  
  In pure object-oriented languages like C# and Java it's quite easy to make sure your class remains valid, but in languages like C++ this can get a bit tricky since there are so many ways to corrupt your data without you even realizing it.

Interesting, you just pass the Player (Character) as a reference into the Spell class! Would I do the same when I am, for example, going through a list of connected players and want to download all data from their stream, for example...


[source lang="csharp"]using System;
using System.Collections.Generic;
using System.Threading;
using System.Net;
using System.Net.Sockets;

namespace OOPTest
{
class Server
{
List clients = new List();
//World world = new World();
public void Start()
{
//Listener.Listen(); start tcp listen thread here for connectting clients
while (true)
{
for (int i = 0; i < clients.Count; i++)
{
Downloader.Download(clients[0]);
}
//world.Update(); update game world
//Uploader.Upload(clients[0]) upload any data to clients from newly updated game world
Thread.Sleep(10);
}
}
}
class Client
{
private TcpClient tcpclient;
private NetworkStream networkstream;
public Client(TcpClient tcpclient, NetworkStream networkstream)
{
this.tcpclient = tcpclient;
this.networkstream = networkstream;
}
public TcpClient Client
{
get { return tcpclient; }
}
public NetworkStream Stream
{
get { return networkstream; }
}
}
static class Downloader
{
static private long bytes_downloaded = 0;
static public void Download(Client client)
{
while (client.Stream.DataAvailable)
{
byte[] read_bytes = new byte[256];
int read_size = client.Stream.Read(read_bytes, 0, read_bytes.Length);
bytes_downloaded += (long)read_size;
//do some processing of received bytes here...
}
}
static public long BytesDownloaded
{
get { return bytes_downloaded; }
}
}
}[/source]

The problem I have here though is how do I allow the static Listener class add any newly received clients to the Server's list?
Thanks for the reply, it really helps
Xanather.

First of all I'm not sure whether you'd want to be using so many static classes in a multithreaded environment like that

Second, I don't see a Listener class defined in the code you provided, but I assume this is what you want:

Since you're using static classes I'll try to explain this with static classes in mind, although I must note that static classes are not considered proper object-oriented structures in most use cases, and they can become quite restrictive later on.

You could add a registerClient method to your server class which accepts a client as input, validates it (class invariant!), and then when validated adds it to the list of clients.

In your listener class you could add a similar function called registerServer, assuming that only 1 server will be attached to 1 client listener at all times. Your listener also validates your server so it's sure that it can safely use it and then sets it as the active server to redirect clients to.

So now when your server starts up you could call Listener.registerServer(this) so that the listener knows which server it should send clients to, and you can run your update loop.

I want to warn you though that I can see quite some issues with this design since both the server and the listener will probably have to work asynchronously, and this design could corrupt your data quite easily.

I didnt finish that source, i just quickly wrote it up and added a few other notes for where things may be, u assumed right with the listener (probably should have said something though xD).
Reply: Ok, well, that makes sense , in this case should I just restrain myself from using a static class? (even though at any 1 time only 1 instance of this class will be created).

Also how could that design corrupt data easily? With all the correct measures and encapsulation methods what could go wrong? Otherwise what would you suggest?

Take a look at this case:

Both your listener thread and your server thread are running. Your server thread is iterating over the list of clients and downloading data. At the exact same time a client connects to your listener, and the listener sends the client to the server, which then adds it to the list. Now I don't know how the provided list class is designed, so I don't know if this really would be an issue (if someone could enlighten me on that, that would be great), but the possibility could arise that your server will be reading from your list while it's being written to. This could result in undefined behaviour, and should be avoided at all costs.

A solution to this would be to add a synchronization mechanism so the server's client list can be updated safely, but implementing such a thing naively will negate any performance gain you get from using multithreading and could even negatively impact your overall performance.

When we look at static classes we can see some of the same issues we see with singleton classes:

-Testing static classes becomes a PITA since you can't do any proper unit tests on them, as they are basically a system for providing procedural functions in an object-oriented environment.
-To avoid data corruption in static classes in a multithreaded environment you'll need to provide mechanisms for safely accessing your static class data if you don't want your code to result in all kind of nasty behaviour. Locking and synchronization mechanisms however can ruin your application's performance though when not done correctly, and other nasties like deadlocks could occur.

Take a look at this case:

Both your listener thread and your server thread are running. Your server thread is iterating over the list of clients and downloading data. At the exact same time a client connects to your listener, and the listener sends the client to the server, which then adds it to the list. Now I don't know how the provided list class is designed, so I don't know if this really would be an issue (if someone could enlighten me on that, that would be great), but the possibility could arise that your server will be reading from your list while it's being written to. This could result in undefined behaviour, and should be avoided at all costs.

A solution to this would be to add a synchronization mechanism so the server's client list can be updated safely, but implementing such a thing naively will negate any performance gain you get from using multithreading and could even negatively impact your overall performance.

When we look at static classes we can see some of the same issues we see with singleton classes:

-Testing static classes becomes a PITA since you can't do any proper unit tests on them, as they are basically a system for providing procedural functions in an object-oriented environment.
-To avoid data corruption in static classes in a multithreaded environment you'll need to provide mechanisms for safely accessing your static class data if you don't want your code to result in all kind of nasty behaviour. Locking and synchronization mechanisms however can ruin your application's performance though when not done correctly, and other nasties like deadlocks could occur.

I see, hmm, surprisingly enough Ive actually thought of this before! Ive never had the problem though as the server thread is sleeping whenever it doesn't need to do work (i.e. waiting for the next 16.6 millisecond game tick) which has made the listener thread have a extremely high chance to add clients during this sleeping period. But your right though, there is a chance a client could be added when the server is already doing processing.

Here is what I am experiencing though at the moment: All this encapsulation and making a class a complete single and separate entity is probably going to reduce actual coding time and increase the time of me thinking about, and implementing, the OOP principles... and I don't like the saying that. But Ive heard many times, in the long term after the application is developed it will help extremely, correct?

I'm still wondering though, how would you tackle the following senario: allow the Download class to communicate to the Messager class (which processes received bytes from clients) which then communicates to the Spell class (as a cast-a-spell message may have been received) which then communicates with the correct Player class to inflict damage to that player? Should I make each class have reference types of the only the other classes in which is MAY communicate to?

Thanks for the replies,
Xanather.