class object
{
private:
D3DXVECTOR2 pos;
float dirAngle;
float rotAngle;
DWORD lastShoot;
public:
//constructors
object();
object(D3DXVECTOR2, float, float, float);
//Set position
void setPos(D3DXVECTOR2);
// This are the add or subtract functions
void add_x(float);
void sub_x(float);
void add_y(float);
void sub_y(float);
void add_dir(float);
void sub_dir(float);
void add_rot(float);
void sub_rot(float);
//This are the retrieve functions
float getDir();
float getRot();
float getLast();
};
object::object()
{
pos = D3DXVECTOR2(0.0f,0.0f);
dirAngle = 0;
rotAngle = 0;
lastShoot = 0;
}
object::object(D3DXVECTOR2 p,
float dir,
float rot,
float last)
{
pos = p;
dirAngle = dir;
rotAngle = rot;
lastShoot = last;
}
void object::setPos(D3DXVECTOR2 p)
{
pos = p;
}
//Change X
void object::add_x(float x)
{
pos.x += x;
}
void object::sub_x(float x)
{
pos.x -= x;
}
//Change Y
void object::add_y(float y)
{
pos.y += y;
}
void object::sub_y(float y)
{
pos.y += y;
}
//Change Dir
void object::add_dir(float dir)
{
dirAngle += dir;
}
void object::sub_dir(float dir)
{
dirAngle -= dir;
}
void object::add_rot(float
//....
//Get Properties
float getDir()
{
return dirAngle;
}
float getRot()
{
return rotAngle;
}
float getLast()
{
return lastShoot;
}
Affecting Private Members
Author
I am creating an object class for my game and well all its data is private(I am still not sure why people keep stuff private, my understanding is it's to keep them safe from being changed unless you want them to be changed), I am new to handling things this way and well I am creating functions to retrieve and change this members but It's getting quite unmanagable.. I am pretty sure I am doing thisn wrong, can someone tell me how to make this simpler?
Your opinion about private members is correct. However the methods you made could be done real simpler(really if you need more operators like multiply/cross/dot). Here's my version of it. I'm not sure if the d3d9x lib comes with vector operations, else you might make a vector class.
class object
{
private:
D3DXVECTOR2 pos;
float dirAngle;
float rotAngle;
DWORD lastShoot;
public:
//constructors
object();
object(D3DXVECTOR2, float, float, float);
D3DXVECTOR2 &GetPos() { return pos; }
float &GetDirAngle() { return dirAngle; }
float &GetRotAngle() { return rotAngle; }
DWORD &GetLastShoot() { return lastShoot; }
};
int main()
{
object obj;
obj.GetPos() = Vector(0,0,0); // You might need some operator for this
obj.GetPos()+=p; // etc...
}
class object
{
private:
D3DXVECTOR2 pos;
float dirAngle;
float rotAngle;
DWORD lastShoot;
public:
//constructors
object();
object(D3DXVECTOR2, float, float, float);
D3DXVECTOR2 &GetPos() { return pos; }
float &GetDirAngle() { return dirAngle; }
float &GetRotAngle() { return rotAngle; }
DWORD &GetLastShoot() { return lastShoot; }
};
int main()
{
object obj;
obj.GetPos() = Vector(0,0,0); // You might need some operator for this
obj.GetPos()+=p; // etc...
}
Here are some tips:
1) Be consistent. Be consistent in everything you do. It will make managing your code so much easier. This means be consistent in your naming convention (either do [addX, subDir, getRot, etc.] or [add_x, sub_dir, get_rot, etc.], but don't do both of them!).
2) Don't be afraid of spelling the whole word out. getDir() and getRot() would make a lot more sense if they were getDirection() and getRotation().
3) (a) Think about something really should be private or public (or protected). Member variables are often made private because of abstraction. When someone is using your class, they don't care about the exact implementation details; that's for you to worry about, not them. That's why you split up the class into "public" and "private" parts. They want your class to do something for them, and as long as it does it, they don't care how it does it. Plus if you ever need to change the internal implementation of the class, you can still keep the public interface, which means that you can change/update your class without breaking existing code.
(b) Another reason for making members private is because sometimes changing one can have a rather large impact. For example, let's pretend you have a class that represents a 2D vector. It has three variables: x, y, and angle. x and y are the (x, y) coordinates of the vector, and angle is the angle the vector makes with the x-axis. If the variables were public and someone were to change x, that could potentially change angle too! So then you decide to make the variables private and use accessor methods (the getters/setters) so that if someone wants to change x, you can also change angle in the accessor method without the user having to worry about manually updating the angle.
4) Realize that your class isn't getting unmanageable (or if you really think it is, identify exactly what part(s) of it is/are unmanageable). Your class is pretty small, and you only have a few accessor methods. You'll be writing much bigger classes with a lot more accessor methods someday. Welcome to the world of programming. (But in terms of manageability, consider visitor's suggestion of renaming your functions. He makes a very good point.)
5) Read up on this. If you have a const object, you can only use its const member functions.
Your class is so simple that if I were you, I'd consider (note I said consider, as in think about it, not necessarily actually do it) making the variables public. However, I'm not entirely sure what you are trying to accomplish with your class, so only make the variables public if it makes sense.
[edit]
About johan postma's post: Hmmm... I'd never thought of that. Cool. Anyway, there really isn't much difference from that code and just making the members public. Personally I'd prefer public members with meaningful, descriptive names over those types of functions. The reason being is getter methods are almost universally implemented in such a way that the variable is meant to be read, not written to. This breaks my #1 rule of being consistent.
[ninja'd++]
[Edited by - MikeTacular on December 20, 2008 12:18:14 PM]
1) Be consistent. Be consistent in everything you do. It will make managing your code so much easier. This means be consistent in your naming convention (either do [addX, subDir, getRot, etc.] or [add_x, sub_dir, get_rot, etc.], but don't do both of them!).
2) Don't be afraid of spelling the whole word out. getDir() and getRot() would make a lot more sense if they were getDirection() and getRotation().
3) (a) Think about something really should be private or public (or protected). Member variables are often made private because of abstraction. When someone is using your class, they don't care about the exact implementation details; that's for you to worry about, not them. That's why you split up the class into "public" and "private" parts. They want your class to do something for them, and as long as it does it, they don't care how it does it. Plus if you ever need to change the internal implementation of the class, you can still keep the public interface, which means that you can change/update your class without breaking existing code.
(b) Another reason for making members private is because sometimes changing one can have a rather large impact. For example, let's pretend you have a class that represents a 2D vector. It has three variables: x, y, and angle. x and y are the (x, y) coordinates of the vector, and angle is the angle the vector makes with the x-axis. If the variables were public and someone were to change x, that could potentially change angle too! So then you decide to make the variables private and use accessor methods (the getters/setters) so that if someone wants to change x, you can also change angle in the accessor method without the user having to worry about manually updating the angle.
4) Realize that your class isn't getting unmanageable (or if you really think it is, identify exactly what part(s) of it is/are unmanageable). Your class is pretty small, and you only have a few accessor methods. You'll be writing much bigger classes with a lot more accessor methods someday. Welcome to the world of programming. (But in terms of manageability, consider visitor's suggestion of renaming your functions. He makes a very good point.)
5) Read up on this. If you have a const object, you can only use its const member functions.
Your class is so simple that if I were you, I'd consider (note I said consider, as in think about it, not necessarily actually do it) making the variables public. However, I'm not entirely sure what you are trying to accomplish with your class, so only make the variables public if it makes sense.
[edit]
About johan postma's post: Hmmm... I'd never thought of that. Cool. Anyway, there really isn't much difference from that code and just making the members public. Personally I'd prefer public members with meaningful, descriptive names over those types of functions. The reason being is getter methods are almost universally implemented in such a way that the variable is meant to be read, not written to. This breaks my #1 rule of being consistent.
[ninja'd++]
[Edited by - MikeTacular on December 20, 2008 12:18:14 PM]
void add_x(float);void sub_x(float);void add_y(float);void sub_y(float);These could probably be condensed to:
void move(D3DXVECTOR2)The thing with making things private is not only to limit who can modify the values but also help the rest of the program not care what the members are at all. For this it might help to think about the class in terms of services that it provides (move) rather than in terms of manipulating the class members (add_x etc).
void add_dir(float);void sub_dir(float);void add_rot(float);void sub_rot(float);For these you might need half the number of functions (substracting is the same as adding a negative value) and again you might rather call them after what they do (e.g rotate, turn) and not emphasize on the member variable that is affected (add_dir, add_rot).
//This are the retrieve functionsfloat getDir();float getRot();float getLast();These should be marked as constant methods since calling them shouldn't modify the instance.
float getDir() const;
Author
I don't really understand how this works/what this does :S.
'class object
{
private:
D3DXVECTOR2 pos;
float dirAngle;
float rotAngle;
DWORD lastShoot;
public:
//constructors
object();
object(D3DXVECTOR2, float, float, float);
D3DXVECTOR2 &GetPos() { return pos; }
float &GetDirAngle() { return dirAngle; }
float &GetRotAngle() { return rotAngle; }
DWORD &GetLastShoot() { return lastShoot; }
};
int main()
{
object obj;
obj.GetPos() = Vector(0,0,0); // You might need some operator for this
obj.GetPos()+=p; // etc...
}'
About the creating a move function, good idea, something like this?
I would like to combine move_forward/backward into one, something like 'move', and the turn_right/left and direct_right/left into 1, something like 'rotate'.
The way it's set up right now direction is used with the move function to change the object's position while rotation changes the sprite's rotation(I noticed something strange, if you add to direction you have to subtract from rotation that's why I am using two different variables... If I tried to use direction for both then the sprite would rotate at the opposite direction from where the object was heading)
How could I do this?
Some questions, a constant variable you can't reset right? Neither can you change its value in any way?(Adding, subtracting)
The suggestion to make my variables global, I am confused aren't global variables bad? Or just in big classes?
'class object
{
private:
D3DXVECTOR2 pos;
float dirAngle;
float rotAngle;
DWORD lastShoot;
public:
//constructors
object();
object(D3DXVECTOR2, float, float, float);
D3DXVECTOR2 &GetPos() { return pos; }
float &GetDirAngle() { return dirAngle; }
float &GetRotAngle() { return rotAngle; }
DWORD &GetLastShoot() { return lastShoot; }
};
int main()
{
object obj;
obj.GetPos() = Vector(0,0,0); // You might need some operator for this
obj.GetPos()+=p; // etc...
}'
About the creating a move function, good idea, something like this?
class object{private: D3DXVECTOR2 position; float direction_angle; float rotation_angle; DWORD time_since_last_shot; public: //constructors object(); object(D3DXVECTOR2, float, float, DWORD); //Set position void set_position(D3DXVECTOR2); // This are the add or subtract functions void move_forward(); void move_backward(); void direct_forward(); void direct_backward(); void turn_forward(); void turn_backward(); //This are the retrieve functions float get_direction() const; float get_rotation() const; float get_time_since_last_shot() const; float get_degrees_to_radians();};object::object(){ position = D3DXVECTOR2(0.0f,0.0f); direction_angle = 0; rotation_angle = 0; time_since_last_shot = 0;}object::object(D3DXVECTOR2 new_position, float new_direction, float new_rotation, DWORD new_time_since_last_shot){ position = new_position; direction = new_direction; rotation = new_rotation; time_since_last_shot = new_time_since_last_shot;}void object::set_position(D3DXVECTOR2 new_position){ position = new_position;}//Movevoid object::move_forward(){ position.x -= 2.0f * sin(direction * get_degrees_to_radians()); position.y -= 2.0f * cos(direction * get_degrees_to_radians());}void object::move_backward(){ position.x += 2.0f * sin(direction * get_degrees_to_radians()); position.y += 2.0f * cos(direction * get_degrees_to_radians());}//Directionvoid object::direct_right(){ direction -= 2.0f; if(direction < 0) direction += 360;}void object::direct_left(){ direction += 2.0f; if(direction > 360) direction -= 360;}//Rotationvoid object::turn_right(){ rotation -= 2.0f; if(rotation < 0) rotation += 360;}void object::turn_left(){ rotation += 2.0f; if(rotation > 360) rotation -= 360;}//Get Propertiesfloat get_direction() const{ return direction;}float get_rotation() const{ return rotation;}float get_time_since_last_shot() const{ return time_since_last_shot;}float get_degrees_to_radians() const{ return 13.14159f / 180.0f;}I would like to combine move_forward/backward into one, something like 'move', and the turn_right/left and direct_right/left into 1, something like 'rotate'.
The way it's set up right now direction is used with the move function to change the object's position while rotation changes the sprite's rotation(I noticed something strange, if you add to direction you have to subtract from rotation that's why I am using two different variables... If I tried to use direction for both then the sprite would rotate at the opposite direction from where the object was heading)
How could I do this?
Some questions, a constant variable you can't reset right? Neither can you change its value in any way?(Adding, subtracting)
The suggestion to make my variables global, I am confused aren't global variables bad? Or just in big classes?
Quote:Original post by johan postmaTake a look at what you have written here, and then ask yourself whether you gain anything by making the variables private.
Your opinion about private members is correct. However the methods you made could be done real simpler(really if you need more operators like multiply/cross/dot). Here's my version of it. I'm not sure if the d3d9x lib comes with vector operations, else you might make a vector class.
class object
{
private:
D3DXVECTOR2 pos;
float dirAngle;
float rotAngle;
DWORD lastShoot;
public:
D3DXVECTOR2 &GetPos() { return pos; }
float &GetDirAngle() { return dirAngle; }
float &GetRotAngle() { return rotAngle; }
DWORD &GetLastShoot() { return lastShoot; }
};
int main()
{
object obj;
obj.GetPos() = Vector(0,0,0); // You might need some operator for this
obj.GetPos()+=p; // etc...
}
You aren't preventing them from being changed by other classes/functions, because they can assign whatever they like to the reference, you aren't hiding implementation details, because a reference must refer to an actual instance, and you can't have side-effects on modification, because you can't detect the modification.
Basically you have added a lot of boilerplate code for zero gain, and (at least in my opinion) at the cost of a lot of readability. Also don't forget that when debugging you probably have compiler optimisations turned off, in which case all these little functions won't be inlined.
Yes, you cannot change a const variable. Which means if someone made a const object of your class, they would not be able to call the move() function on it because that would change the object in some way.
Woah, wait. Nobody said to make your variables global. Global and public are 2 very, very different things. A public variable is a variable stored within a class that can be directly access through an object of that class. A global is a variable that doesn't exist in a class, it exists globally. You don't have to have an object of a class to use a global variable. Public variables aren't always bad. In my complex number class, I made the real and imaginary parts of the complex number public. There are times when public variables are the best tool, and there are times when they are not.
As for the code that you don't understand, it works because it uses references. Wikipedia and the C++ FAQ Lite explain references better than I could. But I wouldn't use that code if I were you.
Now about combining your functions: what do you recognize in common between the functions? They're exactly the same, except for a +/-. What if you had something like this:
Note I only have to store one angle and I now only have a few functions. If you wanted to move the object forward 10 units, you just call move(10.0f). If you want to move the object backwards 10 units, just call move(-10.0f). What do you think of my above code? Do you have any questions about it?
Good job on adding const to your functions and being consistent with your names!
[disclaimer]
I haven't tested the code above, but I'm pretty sure it's right. I don't know how D3DXVECTOR2 operates, so you may have to adjust the code so it properly accesses the correct members
[/disclaimer]
[edit]
I noticed you had swapped sin and cos in your move function. cos should be used for the x-axis, and sin should be used for the y-axis. Also, in my code, I didn't implement any functions to set the position and angle. I figured you would be able to do this fairly easily on your own.
Woah, wait. Nobody said to make your variables global. Global and public are 2 very, very different things. A public variable is a variable stored within a class that can be directly access through an object of that class. A global is a variable that doesn't exist in a class, it exists globally. You don't have to have an object of a class to use a global variable. Public variables aren't always bad. In my complex number class, I made the real and imaginary parts of the complex number public. There are times when public variables are the best tool, and there are times when they are not.
As for the code that you don't understand, it works because it uses references. Wikipedia and the C++ FAQ Lite explain references better than I could. But I wouldn't use that code if I were you.
Now about combining your functions: what do you recognize in common between the functions? They're exactly the same, except for a +/-. What if you had something like this:
class Object{ public: // This moves the object in the direction it's already facing. // A positive displacement is forward, and a negative displacement // is backwards. void move(float displacement); // This moves the object according to the variable 'displacement'. // Essentially, just add it do your current position void move(D3DXVECTOR2 displacement); // This rotates the object theta degrees. Remember that a positive // rotation is a counterclockwise rotation. void rotate(float theta); float getAngle() const; D3DXVECTOR2 getPosition() const; private: // You could make the variables public. It's up to you. float angle; D3DXVECTOR2 position; // This really shouldn't be a member fuction, but I'm putting it here // just for this sample code float degreesToRadians(float degrees) const;};void Object::move(float displacement){ position.x += displacement * std::cos(degreesToRadians(angle)); position.y += displacement * std::sin(degreesToRadians(angle));}void Object::move(D3DXVECTOR2 displacement){ position += displacement; // The above is the same as doing: // position.x += displacement.x; // position.y += displacement.y;}void Object::rotate(float theta){ angle += theta;}float Object::getAngle() const{ return angle;}D3DXVECTOR2 Object::getPosition() const{ return position;}float Object::degreesToRadians(float degrees){ const float pi = 3.1415926535897932384626433832795f; return degrees * pi / 180.0f;}Note I only have to store one angle and I now only have a few functions. If you wanted to move the object forward 10 units, you just call move(10.0f). If you want to move the object backwards 10 units, just call move(-10.0f). What do you think of my above code? Do you have any questions about it?
Good job on adding const to your functions and being consistent with your names!
[disclaimer]
I haven't tested the code above, but I'm pretty sure it's right. I don't know how D3DXVECTOR2 operates, so you may have to adjust the code so it properly accesses the correct members
[/disclaimer]
[edit]
I noticed you had swapped sin and cos in your move function. cos should be used for the x-axis, and sin should be used for the y-axis. Also, in my code, I didn't implement any functions to set the position and angle. I figured you would be able to do this fairly easily on your own.
Author
It looks much better already!
I do have a couple of trivial questions.
I am still wondering if a single rotation variable will do, as I mentioned before trying to use the same value with the movement calculation and the rotation calculation had the sprite rotate in the opposite direction from where it was heading(I had to make a second variable with an opposite value to the rotation's to apply on the rotation calculation.. they became direction and rotation respectively).
I noticed you didnt made a check when changing rotation if you had gone below 0 or over 360 degrees to set it back(+360 to 0, -0 to 360) is this because the compiler will do that for you? Like turning 380 degrees into 20/ 720 into 360..?
Won't such a large amount of numbers on the pi definition cause problems? The calculation is made quite often during the game.
I do have a couple of trivial questions.
I am still wondering if a single rotation variable will do, as I mentioned before trying to use the same value with the movement calculation and the rotation calculation had the sprite rotate in the opposite direction from where it was heading(I had to make a second variable with an opposite value to the rotation's to apply on the rotation calculation.. they became direction and rotation respectively).
I noticed you didnt made a check when changing rotation if you had gone below 0 or over 360 degrees to set it back(+360 to 0, -0 to 360) is this because the compiler will do that for you? Like turning 380 degrees into 20/ 720 into 360..?
Won't such a large amount of numbers on the pi definition cause problems? The calculation is made quite often during the game.
class object{private: D3DXVECTOR2 position; float rotation; DWORD time_since_last_shot; public: //constructors object(); object(D3DXVECTOR2, float, DWORD); // This are the add or subtract functions void move(float); void rotate(float); //This are the retrieve functions D3DXVECTOR2 get_position() const; float get_rotation() const; DWORD get_time_since_last_shot() const; float degrees_to_radians();};object::object(){ position = D3DXVECTOR2(0.0f,0.0f); rotation = 0; time_since_last_shot = 0;}object::object(D3DXVECTOR2 new_position, float new_rotation, DWORD new_time_since_last_shot){ position = new_position; rotation = new_rotation; time_since_last_shot = new_time_since_last_shot;}//Movevoid object::move(float displacement){ position.x += displacement * cos(degrees_to_radians(rotation)); position.y += displacement * sin(degrees_to_radians(rotation));}//Directionvoid object::rotate(float theta){ rotation += theta; if(rotation < 0) rotation += 360; if(rotation > 360) rotation -= 360;}//Get PropertiesD3DXVECTOR2 get_position() const{ return position;}float get_rotation() const{ return rotation;}DWORD get_time_since_last_shot() const{ return time_since_last_shot;}float degrees_to_radians(degrees){ const float pi = 3.1415926535897932384626433832795f; return degrees * pi / 180.0f;}
Yes, you're right, I should have included a check when rotating to wrap the angle from [0, 360] degrees. The computer/compiler will not do it automatically.
I'm still not sure why you had to use two angles. You shouldn't need more than one. I think maybe the problem came from using sin/cos incorrectly. Or possibly applying the angle incorrectly when rendering. You originally had
position.x -= 2.0f * sin(direction * get_degrees_to_radians()); // should be cos
position.y -= 2.0f * cos(direction * get_degrees_to_radians()); // should be sin
So I think that may have been the problem. Try your new code and see if you still have the same problem. If you do, then there is probably something wrong in your rendering step and we can take a look at that code.
[edit]
Wait. The move function I wrote (which is in your new code) is for regular Cartesian coordinates, like this:
However, if you're working in 2D, you might be using screen coordinates, like this:
If you are using screen coordinates, then some adjustments will have to be made. Let me know if you are using screen coordinates and I'll help you out.
I'm still not sure why you had to use two angles. You shouldn't need more than one. I think maybe the problem came from using sin/cos incorrectly. Or possibly applying the angle incorrectly when rendering. You originally had
position.x -= 2.0f * sin(direction * get_degrees_to_radians()); // should be cos
position.y -= 2.0f * cos(direction * get_degrees_to_radians()); // should be sin
So I think that may have been the problem. Try your new code and see if you still have the same problem. If you do, then there is probably something wrong in your rendering step and we can take a look at that code.
[edit]
Wait. The move function I wrote (which is in your new code) is for regular Cartesian coordinates, like this:
However, if you're working in 2D, you might be using screen coordinates, like this:
If you are using screen coordinates, then some adjustments will have to be made. Let me know if you are using screen coordinates and I'll help you out.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
