Composition - accessing & updating members

[jgranga171]

Hello everyone!

I am trying to figure out what to do here. I'm new at programming and this particular situation has me stumped. Here is what I'm trying to figure out:

I have two classes. One is class Coordinate, and one is class Ship. A Coordinate object is made up of an x, y, and z value (all doubles) and is constructed like this (x, y, z). A Ship object is made up of a coordinate, and an x_inertia, y_inertia, and z_inertia value (also doubles).

Coordinate(double x, double y, double z)
Ship(Coordinate, x_inertia, y_inertia, z_inertia)

I'm using composition because a Ship (has-a) coordinate.

I want to be able to update a Ship's location (Coordinate) using the inertia values via an updateLocation() function.

ex: Ship((5,5,5)1,1,1) ->after updateLocation-> Ship((6,6,6)1,1,1)

I have the code written, but since x, y, and z of class coordinate are protected, I cannot update the locations of the ships unless I "comment" out the "protected" list (thus making the variables "public").

I'm obviously missing something, and I know it's probably simple, but I don't know what. Here is my updateLocation() function:

Ship::Ship(Coordinate position, double new_x, double new_y, double new_z)
: location(position),
x_inertia(new_x),
y_inertia(new_y),
z_inertia(new_z)
{
}

void Ship::updateLocation()
{
location.x += x_inertia;
location.y += y_inertia;
location.z += z_inertia;
}

Any advice? Anything else you need to see?

[Richard.J]

Your Coordinate class could provide a public method to set x,y and z. You can then call that method inside your updateLocation

[Chris_F]

I have the code written, but since x, y, and z of class coordinate are protected, I cannot update the locations of the ships unless I "comment" out the "protected" list (thus making the variables "public").

Well, maybe you should ask yourself the real important question of why are the values of Coordinate marked as private? I can think of no reason.

You are better off making Coordinate a struct with publicly accessible x, y and z members. There is no major difference between the struct and class keyword in C++, but using struct for POD types and class for complex objects with invariants is pretty standard.

Maybe you have some experience with Java or other OOP language and have the impression that public members is bad and accessor members are good? Well that isn't always the case. Hiding data behind accessors is only necessary if your class needs to maintain an invariant, something a basic coordinate/vector class does not need to do.

In addition, you may consider using the OpenGL Mathematics library. http://glm.g-truc.net/

Don't let the name fool you. It's has no dependencies on OpenGL or graphics at all. It's a basic, header-only, vector library, and you could use it like so:

using glm::vec3;

class Ship
{
public:
    Ship(vec3 position, vec3 inertia)
        : m_position(position), m_inertia(inertia)
    {}
    void UpdateLocation()
    {
        m_position += m_inertia;
    }
private:
    vec3 m_position, m_inertia;
};

//e.g.

Ship myship(vec3(0.0f, 0.0f, 0.0f), vec3(1.0f, 0.0f, 0.0f));
myship.UpdateLocation(); //moves positively in x direction by 1

[D_Drmmr]

Well, maybe you should ask yourself the real important question of why are the values of Coordinate marked as private? I can think of no reason.
...
Maybe you have some experience with Java or other OOP language and have the impression that public members is bad and accessor members are good? Well that isn't always the case. Hiding data behind accessors is only necessary if your class needs to maintain an invariant, something a basic coordinate/vector class does not need to do.

In this case, the advantage of a well encapsulated class is to provide an interface that is harder to use incorrectly. If both the position and inertia are kept as coordinates, and the coordinate class has overloaded operators (just like the example code you posted), you can avoid these kinds of mistakes:

location.x += x_inertia;
location.x += y_inertia;
location.z += z_inertia;

[Chris_F]

In this case, the advantage of a well encapsulated class is to provide an interface that is harder to use incorrectly. If both the position and inertia are kept as coordinates, and the coordinate class has overloaded operators (just like the example code you posted), you can avoid these kinds of mistakes:

location.x += x_inertia;
location.x += y_inertia;
location.z += z_inertia;

You don't need encapsulation to provide a robust interface for a coordinate class. Like I said, It's not an invariant, and the glm::vec3 class I mentioned is a perfect example. Encapsulating it is just plain silly.

[nuzzle]

I'm obviously missing something, and I know it's probably simple, but I don't know what.

You're missing that since the Location class is to be used in has-a relationships and since you want it to be mutable you must make x, y and z publicly accessible for reading and writing. Either you declare the variables public, or you declare them private but supply a public getter/setter method pair for each of them.

Or as an alternative you can make Location immutable. Then the variables are made private with a public getter for each. Immutable means an object never changes after creation so in this case a Location object is "updated" simply by being replaced by a newly created one with the new variable values.

Having protected access only makes sense in an is-a situation where Location is to be inherited.

[nuzzle]

Hiding data behind accessors is only necessary if your class needs to maintain an invariant, something a basic coordinate/vector class does not need to do.

How do you know that?

The level of encapsulation and the extent of mutability/immutability are design issues.

[Chris_F]

How do you know that?

The level of encapsulation and the extent of mutability/immutability are design issues.

Because a vector is a basic type. If your Ship class needs to restrict the valid states of position and inertia then IT should be maintaining invariants by encapsulating the vectors.

[nuzzle]

Because a vector is a basic type.

And where did you get the idea that "basic types" (whatever that means) cannot be immutable?

[jgranga171]

Wow. You guys are great! I VERY MUCH appreciate all of your suggestions. In the end, I went with Richard.J's and nuzzle's suggestions, where I created public "getX(), getY(), and getZ() functions within class Coordinate for use in updateLocation() of class Ship.

***EDIT: Disregard this statement. I was mistaken -> "Now, I took this route because (Chris_F, speaking to you) we weren't taught anything about vectors in my course, only arrays. I'm sure that using vectors would be more efficient, but the fact of the matter is, I have no idea how to use them. I WILL be going over it on my own." ***

D_Drmmr, thank you for the great advice, although for the project, the inertias are separate double variables specifically, meaning they cannot be initialized as a coordinate.

Now, one more question... encapsulation - not sure I understand this term fully yet. Can you guys explain that? Is it something that I'm already using and I'm just too dense to see that? Ha! Attached are text files of my updated code. I was going to just copy and pasted into the thread, but it seems to me to be too long to do that without annoying everyone. Keep in mind, I'm sure there are a plethora of better ways to write it, but what you see is what the project guidelines called for. However, if you see that there are underlying errors or better ways of doing it, feel free to blast me for it. Thanks guys!

[jgranga171]

Additionally, here is the output I'm getting, and the project guidelines that I had to create the project from:

Output:
Round 0:

No collisions!

Round 1:

No collisions!

Round 2:

No collisions!

Round 3:

Collision between ship 1 at (5, 5, 5) and ship 2 at (5, 5, 5)

Round 4:

Collision between ship 0 at (4, 4, 4) and ship 2 at (4, 4, 4)

Round 5:

Collision between ship 0 at (5, 5, 5) and ship 1 at (5, 5, 5)

Round 6:

No collisions!

Round 7:

No collisions!

Round 8:

No collisions!

Round 9:

No collisions!
-----------------------------------------
Guidelines:


Create a new class Ship that uses the class Coordinate as its location in a game board.
• The program must implement the detectCollisions function as defined in the pa10-skeleton.cpp (I had to define this function myself) to simulate game collision detection.
• Ships have a Coordinate, a x_inertia, a y_intertia, and a z_inertia
• All inertias are double types (and are private)
• Ships can provide their coordinate with the getLocation()method
• Ships can update their location by one turn with the updateLocation() method

[Chris_F]

Now, I took this route because (Chris_F, speaking to you) we weren't taught anything about vectors in my course, only arrays. I'm sure that using vectors would be more efficient, but the fact of the matter is, I have no idea how to use them.

You completely misunderstood me. The vector you are thinking of is the dynamic container class that is a part of the standard template library. IMO calling it "vector" was a dumb idea, since obviously it has generated confusion here. Vector is a term used in mathematics and physics. A point (or coordinate) is a location, and a vector is a combination of direction and magnitude. You don't realize it, but you are already using vectors! What do you think your inertia values represent? That's right, it's a vector.

class Vector
{
    //This class is a three dimensional vector
    float x, y ,z;
};

Understand? The glm vector class is just a replacement for your coordinate class, with a different name but the same function. I recommended it because it already implements things like operator overloading, so that you can easily perform mathematical operations with it, like adding your inertia to your position to calculate your new position. There is no sense re-inventing the wheel when a good library already exists. The example I gave you is very straight forward.

And where did you get the idea that "basic types" (whatever that means) cannot be immutable?

How is "could it be" relevant? The question is should it be.

[nuzzle]

How is "could it be" relevant? The question is should it be.

Yes it's a relevant design question to ask to what extent a type should be immutable. It's not like you seem to suggest that certain types shouldn't be immutable.

[Chris_F]

Yes it's a relevant design question to ask to what extent a type should be immutable. It's not like you seem to suggest that certain types shouldn't be immutable.

And what exactly do you gain by making your vector/coordinate class immutable? Are you trying to advocate an immutable vector class, or are you simply determined to make sure he considers it?

Vectors aren't exactly anything new. HLSL, GLSL, Cg, CUDA and OpenCL all have vectors as primitive data types, and pretty much every 3d, game or math library has them, and they are almost always implemented the exact same: as POD. So if you are privy to something on the subject of vectors, then by all means share.

[jgranga171]

Chris_F,

OOOOOOOOOOOH! Ha! I apologize. Now going back and reading your OP with a clearer mindset, it makes perfect sense what you were saying. I'm new! I should've cued in on that a while ago, as I most certainly know what a 'vector' is. Thanks again!