Sign in to follow this  

what to do when 2 constructors of the same class take the same paramaters?

This topic is 4711 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

hi, i have run into a slight design problem and im curious how you guys would handle it. im working on a 2d online RPG, and im implementing the SMG weapon into the game. anyway, basically, this requires me to put a second constructor in my Bullet class, which takes velocity as a parameter. this is because normally a projectile will calculate its own velocity, however, the SMG Weapon fires a 3 shot burst, and in order to know where to place the 2 trailing bullets, the SMG must calculate the velocity. since i dont want to calculate velocity twice each time an SMG is fired, im going to give my Bullet class a second constructor which takes velocity. anyway, the 2 constructors look like this:
Bullets::Bullets(float start_x, float start_y,float x_vel,  float y_vel, Character *shooter, Uint32 timestamp)
and,
Bullets::Bullets(float start_x, float start_y,float dest_x,  float dest_y, Character *shooter, Uint32 timestamp)
so, are there any clever tricks to make this code compilable? i guess i could just give the first constructor a boolean saying "bool velocity_given", and in all cases set that to true, however, this seems kind of ugly.. thanks for any help.

Share this post


Link to post
Share on other sites
I've seen velocity represented as a Point structure with an x and y parameter inside of it.

Just out of curiosity why are there changing bullet velocity? I'll be writing similiar projects in the future and am just curious why it isn't constant (not questioning your design just trying to understand why it may be necessary).

Share this post


Link to post
Share on other sites
bullets can fire in 360 degrees, any direction. velocity must be calculated with each shot.

i dont have a Point class, and dont want to implement one either. thats something that i should have done months ago. but, even then, this would lead to the same problem, unless i did something kind of ugly and had one ctor take a Point and the other take 2 floats.

Share this post


Link to post
Share on other sites
I would suggest using different functions:

// Needed Data for a bullet
Bullets::Bullets(float start_x, float start_y,Character *shooter, Uint32 timestamp)

void Bullet::SetVel( float xVel, float yVel );
void Bullet::SetDest( float destX, float destY );




The other option is to pass *all* the variables into it along with the a bool.


Bullets::Bullets(float start_x, float start_y,float x_vel, float y_vel, float destX, float destY, Character *shooter, Uint32 timestamp, bool calculatesVel = false)




So if you want to create a bullets then:

// Uses Velocity passed in.
Bullet B = new Bullet( x,y,xVel,yVel, 0,0 shooter, time );

// True means calculate velocity from x/y Pos and x/y Dest
Bullet B = new Bullet( x, y, 0, 0, xDest, yDest, shooter, time, true );



Share this post


Link to post
Share on other sites
Quote:
Original post by graveyard filla
bullets can fire in 360 degrees, any direction. velocity must be calculated with each shot.
Your two constructors are functionally identical. The one takes an explicit (2D) vector velocity, the other takes a (2D) vector direction, from which the velocity is computed. In essence, they both take a vector velocity specification.

Your bullet should be told the direction and speed it should travel at, and nothing more.

Share this post


Link to post
Share on other sites
Hi,

Quote:
Original post by graveyard filla
bullets can fire in 360 degrees, any direction. velocity must be calculated with each shot.
i dont have a Point class, and dont want to implement one either. thats something that i should have done months ago.


Well, you should do it now then. The more you wait, the bigger the problem will be. Points and vectors are rather simple objects, the code is straightforward. You can update older code later - you don't have to make your code Point-class aware at the time you are writing the Point class. Do a progressive change - but fo it.

Quote:
Original post by graveyard filla
but, even then, this would lead to the same problem, unless i did something kind of ugly and had one ctor take a Point and the other take 2 floats.


I guess it would bo a better idea. Having a Point class and a Vector class don't hurt and may allow you to write better code:

Bullets::Bullets(const Point& start, const Point& dest,
Character *shooter, Uint32 timestamp)
{
}
Bullets::Bullets(const Point& start, const Vector& velocity,
Character *shooter, Uint32 timestamp)
{
}

Using them:

Bullet bullet0(Point(x,y), Point(xdest, ydest), shooter, ts);
Bullet bullet1(Point(x,y), Vector(xdest-x, ydest-y), shooter, ts);


Quote:
Original post by graveyard filla
so far im thinking the best idea is to just re-order the parameters, e.g., put Character* shooter as the first parameter in the first constructor version. i guess i was just hoping there would be some clever trick for this.


Don't do this: having the same parameters in different order may produce ugly code (and therefore eye cancer).

Anyway, Great Old One Oluseyi probably got it right: is there a conceptual (read: not related to implementation) need for these 2 different-but-not-so-different ctors?

Regards,

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
I would probably create enum types with a single value to use to differentiate constructor calls as so:

enum WithVelocityType { WithVelocity };
enum WithDestinationType { WithDestination };

class Bullet
{
Bullet( WithVelocityType, float x, float y, float dx, float dy, (...) );
Bullet( WithDestinationType, float x, float y, float dest_x, float dest_y, (...) );
};

Bullet b1 = new Bullet( WithVelocity, x, y, dx, dy, (etc.) );
Bullet b2 = new Bullet( WithDestination, x, y, x2, y2, (etc.) );

Then the choice of constructor is still decided at compile-time.

Share this post


Link to post
Share on other sites
You could give your bullet a direction vector of unit length and a speed coefficient. You would only need one constructor:

Bullets::Bullets(float start_x, float start_y, float dest_x, float dest_y, Character *shooter, Uint32 timestamp, float speed_coefficient = 1.0f);

The default parameter means you can state the speed coefficient explicitly, but if you don't, it defaults to 1.0. The direction vector would be calculated from the source and destination values then normalized (made unit length). Then whenever you move your bullet, you multiply the components of direction vector by the speed coefficient and add them to the position vector.

Hope that made sense [smile]

Share this post


Link to post
Share on other sites
Quote:
Original post by Emmanuel Deloget

Well, you should do it now then. The more you wait, the bigger the problem will be. Points and vectors are rather simple objects, the code is straightforward. You can update older code later - you don't have to make your code Point-class aware at the time you are writing the Point class. Do a progressive change - but fo it.


eh, like i said, its just too late and not worth it for me. this project is 9 months in development and 2 seperate projects (client and server). i really dont feel like going through 20k lines of code looking for where i used floats and switch them to Points/Vectors [grin]. and, i dont want to just make changes as i go along..

thanks everyone for your suggestions.

Share this post


Link to post
Share on other sites
Quote:
Original post by Oluseyi
Your bullet should be told the direction and speed it should travel at, and nothing more.


And probably a point of origin.

And some constructive criticism: delta_x and delta_y are not the best way to represent velocity. Velocity should really be just one value. This way, you can scale the one value to the distance the bullet should travel between frames, then calculate where the bullet would be along the path. You would have to do this each frame, but that is the only way to maintain gameplay independent of frame rate. You could use a vector for this. Each frame, the vector's starting point is the bullets position. It's direction is known. It's magnitude will be calculated based on the velocity and time since the bullet's last update. It is then easy to find the ending point of the vector and make this the bullet's new position.

EDIT: Previous poster beat me to it.

Share this post


Link to post
Share on other sites
Quote:
Original post by Oluseyi
Your two constructors are functionally identical. ... In essence, they both take a vector velocity specification. ...


Since, the solution was already stated, I'll just second it. Do what Oluseyi suggests.

Your problem is not uncommon. Much of the time, removing one of the constructors is the right thing to do. Sometimes that doesn't work, and the next thing to try is changing the form of the parameters. Changing the order is a bad idea. Omitting parameters and requiring them to be set after the constructor is called is also a bad idea.

Share this post


Link to post
Share on other sites
Here is a solution of last resort. Now, I know it's not a great example, but it illustrates the solution.

You want to create a person given either a height or a weight:
    class Person
{
Person( float height );
Person( float weight ); // uh oh
};
Here is a solution:

struct Height
{
Height( float h ) : value( h ) {}
float value;
};
struct Weight
{
Weight( float w ) : value( w ) {}
float value;
};
class Person
{
Person( Height const & h ) { /* do something with h.value */ }
Person( Weight const & w ) { /* do something with w.value */ }
};

Person Adam( Height( 2.0f ) );
Person Eve( Weight( 50.f ) );

Share this post


Link to post
Share on other sites
Quote:
Original post by graveyard filla
i actually decided to just change the order of the parameters.

why is it such a bad idea? you look at the CTOR, and you see what parameters it takes, so you know what it does. yes, its ugly, but, IMO, cleaner then the other solutions offered.


The solution is to write functions with real, distinct names, and have them call the constructor, while having the constructor take as many parameters are necessary to fully specificy the object. Having your constructor do different things based on just parameter ordering is an unspeakable horror.

class Bullets
{
public:
Bullets(float start_x, float start_y,
float x_vel, float y_vel,
Character *shooter, Uint32 timestamp);

static Bullets CreateFromVelocity(float start_x, float start_y,
float x_vel, float y_vel,
Character *shooter, Uint32 timestamp)
{
return Bullets(start_x, start_y, x_vel, y_vel, shooter, timestamp);
}


static Bullets CreateFromEndpoint(float start_x, float start_y,
float dest_x, float dest_y,
Character *shooter, Uint32 timestamp)
{
float x_vel = dest_x - start_x;
float y_vel = dest_y - start_y;
float mag = hypot(x_vel, y_vel);

x_vel /= mag;
y_vel /= mag:

return Bullets(start_x, start_y, x_vel, y_vel, shooter, timestamp);
}
};

Share this post


Link to post
Share on other sites
Quote:
Original post by graveyard filla
i actually decided to just change the order of the parameters.

why is it such a bad idea?

Its a solution and perhaps this is all a lot of fuss over nothing. However, it's a bad idea because it's too easy to put the parameters in the wrong order and call the wrong constructor.

Differentiating between functions simply by the order of parameters is not something you want to do everywhere, is it? In other words, it isn't a good idea, is it?

Share this post


Link to post
Share on other sites
Quote:
Original post by Fruny
The solution is to write functions with real, distinct names, and have them call the constructor, while having the constructor take as many parameters are necessary to fully specificy the object. Having your constructor do different things based on just parameter ordering is an unspeakable horror.

Seconding Fruny. While it would be nice to have "named constructors," they do not exist in C++. One of the best alternatives is to just have a properly named function return an instance of the type to make your intentions clear. What's also good about this solution is that when calling such a function and using the returned object to directly initialize an object of the same type, the temporary you create inside the function can actually legally be optimized out in an optimizing compiler such that the function will directly work with the object being initialized rather than make a copy or two (so on an optimizing compiler there can potentially be no extra overhead for using a separate function instead of a constructor).

The only problem is that even though the copy operation can legally be optimized away, your type must still have a copy constructor. So, if your type is designed to be noncopyable, that solution goes out the window and you're forced into doing things such as explicitly using descriptive dummy parameters (which can also be optimized away).

Share this post


Link to post
Share on other sites
Quote:
Original post by Fruny
... Having your constructor do different things based on just parameter ordering is an unspeakable horror.

LOL. Where on the scale would you find "evil", "spawn of Satan" and "your worst nightmare" in relation to "unspeakable horror"?

Share this post


Link to post
Share on other sites
Pretty much seconding Fruny also. Okay, I guess that's thirding...

What are the actual definitions (code) of these constructors, and what are the member variables for 'Bullets'?
(I wouldn't have made Bullet plural for the class name btw)

Share this post


Link to post
Share on other sites
Ok, horrible advice I'm going to give but hey,

Do whatever works so long as you ship product and you can understand how it works. If you're using VS.net, make sure that you at least put in comments before the constructors so that intellisense will tell you "this is the constructor which takes..."

Never trust anyone who hasn't finished a project... Regardless of the badness of design, I'd probably just do a hack [like reorder the parameters] for now and add a '#pragma message("todo : fix me")' and try to eventually get around to fixing it but that's just me. This seems like the simplest solution because you've obviously got one type of constructor which is used everywhere already and another which you're just introducing, so why not make the syntax for this new one different. Or get the constructor for the first bullet to construct the other two bullets and assign values directly.

Overall, I don't think that there is a particularly clean way to solve your problem, other than calculating velocity / direction 3 times without resorting to fixing code in other areas, but I'm assuming that you haven't taken perfect design decisions in the first place to get to the point where you 'have to look through 20k lines of code' not knowing where the weapon stuff would be called... so I say, have a hack solution [which at least works, and fix it later, I'm assuming that this new constructor would only have limited use through the rest of the application [ie in the first constructor or the SMG::FireWeapon() function or something]]. You'll be glad you did, when you're working on a far more complete project, learning new things, rather than refactoring old work in pursuit of a perfect design.

//End bad advice rant

CJM

Share this post


Link to post
Share on other sites
Quote:
Original post by JohnBolton
Quote:
Original post by Fruny
... Having your constructor do different things based on just parameter ordering is an unspeakable horror.

LOL. Where on the scale would you find "evil", "spawn of Satan" and "your worst nightmare" in relation to "unspeakable horror"?

(Good) Bunnies - Evil - Your Worst Nightmare - Spawn of Satan - Altering parameter order as a hack - Azathoth (Not good)

Share this post


Link to post
Share on other sites
Excerpt from evil.h, an actual header in our local toolkit. The header is intended to encapsulate all those evil bit-twiddling functions (e.g. fast inverse square root) you might be tempted to use elsewhere, to keep code clean.


#define EVIL_WHY_ARE_YOU_EVEN_BOTHERING_WITH_THIS (-1)
#define EVIL_NOT_AT_ALL 0
#define EVIL_DEAD 1
#define EVIL_DEAD_PART_II 2
#define EVIL_DEAD_ARMY_OF_DARKNESS 3
#define EVIL_THE_DIET_COKE_OF_EVIL 5
#define EVIL_KNIEVEL 6
#define EVIL_MCBRIDE 7
#define EVIL_QUASI_EVIL 10
#define EVIL_PSEUDO_EVIL 20
#define EVIL_SNIFFLES 50
#define EVIL_PAPER_CUT 75
#define EVIL_UNUSED_EVIL_LEVEL_TO_TAKE_UP_SPACE 99
#define EVIL_SNOWBOARDING 100
#define EVIL_EVIL 250
#define EVIL_HOMER 350
#define EVIL_AND_WE_MEAN_EVIL 500
#define EVIL_FRUITS_OF_SATAN 666
#define EVIL_YOUR_WORST_NIGHTMARE 999
#define EVIL_THE_SICKNESS 1000
#define EVIL_STENCIL_BUFFER 1024
#define EVIL_CTHULHU 4096
#define EVIL_UNSPEAKABLE_HORROR 8192
#define EVIL_BROGANIZATION 65536
#define EVIL_FAST_AND_WRONG_BUT_WHO_CARES 314150



The evil level determines whether the hacked-up functions or the slower-but-sane versions are used. Not all evil levels are meaningful. But that's ok, it's evil.h after all.

Share this post


Link to post
Share on other sites
Quote:
#define EVIL_THE_DIET_COKE_OF_EVIL 5


LMFAO. I'd agree with that scale of wickedness except I'd have to change this one to :

#define EVIL_THE_DIET_DECAFFEINATED_SURGAR_FREE_SAMS_CHOICE_GRAPE_SODA_OF_EVIL

Share this post


Link to post
Share on other sites

This topic is 4711 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this