C++ Question Breaking a monster-sized class into multiple sub-classes

Hlynkacg

Aspiring rocket scientist
Addon Developer
Tutorial Publisher
Donator
Joined
Dec 27, 2010
Messages
1,870
Reaction score
3
Points
0
Location
San Diego
I'm not sure about first error(show that particular code line)

this one.

Code:
class SpiderLEM: public VESSEL3 
{
public:
	SpiderLEM (OBJHANDLE hVessel, int flightmodel);
	~SpiderLEM ();

	[COLOR="Red"]LEM_AP autopilot; // Autopilot class [/COLOR]

	// Vessel functions...


...but the second one is because I forgot to mention one important thing, sorry.
When some class has a member that is another class object then you have to call that member class constructor in initialization list of class that holds it, unless the member class has default constructor.[/code]

Code:
somewhere in SpiderLEM.cpp file
SpiderLEM::SpiderLEM(/*parameters*/) : autopilot(/*parameters*/){
// other constructor's instructions
}


So how/where would I call that?

Do I just copy/paste LEM_AP's constructor (currently blank) into SpiderLEM.cpp?

Do I call it from within SpiderLEM's constructor? if so what should the "parameters" be?


And a seperate but related question...

Can two classes connected in this way have functions or variables that share a name?

i.e. will it cause problems if there is variable called "pitch" in both classes?
 
Last edited:

marooder86

Donator
Donator
Joined
Dec 1, 2010
Messages
188
Reaction score
3
Points
33
Location
London
this one.

Code:
class SpiderLEM: public VESSEL3 
{
public:
	SpiderLEM (OBJHANDLE hVessel, int flightmodel);
	~SpiderLEM ();

	[COLOR="Red"]LEM_AP autopilot; // Autopilot class [/COLOR]

	// Vessel functions...
This is Ok, so the problem lies elsewhere, remove forward declaration of LEM_AP class if there's any and remember about header guards


So how/where would I call that?
Where I told you to do so, in SpiderLEM's constructor initialization list

Do I just copy/paste LEM_AP's constructor (currently blank) into SpiderLEM.cpp?
No, LEM_AP's constructor implementation should be in LEM_AP's .cpp file

Do I call it from within SpiderLEM's constructor? if so what should the "parameters" be?
Yes, as I said you call it from SpiderLEM's constructor initialization list and the parameters should be the ones you've declared
i.e if this is LEM_AP's constructor declaration and implementation
Code:
****************
in LEM_AP header file
class LEM_AP {
public:
    LEM_AP(int mode); // just example parameter

private:
   int m_mode;
};
****************
in LEM_AP cpp file
LEM_AP::LEM_AP(int mode)
{
   m_mode = mode;
}

then
Code:
****************
in SpiderLEM header file

#include "LEM_AP.h"
class SpiderLEM 
{
public;
   SpiderLEM(OBJHANDLE hVessel, int flightmodel, int autopilot_mode);

  LEM_AP autopilot;
};

****************
in SpiderLEM cpp file

SpiderLEM(OBJHANDLE hVessel, int flightmodel, int autopilot_mode)
    : autopilot(autopilot_mode)
{}

And a seperate but related question...
Can two classes connected in this way have functions or variables that share a name?
Yes they can because the scope will be different.
 
Last edited:

Hlynkacg

Aspiring rocket scientist
Addon Developer
Tutorial Publisher
Donator
Joined
Dec 27, 2010
Messages
1,870
Reaction score
3
Points
0
Location
San Diego
This is what I have

Class def in SpiderLEM.h
Code:
class SpiderLEM: public VESSEL3 
{
public:
	SpiderLEM (OBJHANDLE hVessel, int flightmodel);
	~SpiderLEM ();

	LEM_AP autopilot; // Autopilot class

// LEM Functions and variables...

Constructor in SpiderLEM.cpp
Code:
SpiderLEM::SpiderLEM (OBJHANDLE hVessel, int flightmodel)
	: VESSEL3 (hVessel, flightmodel)
	[COLOR="Red"]: autopilot ()[/COLOR]
{
// Set starting values...

Note: ": autopilot()" is flagged as missing a semi-colon, adding a semi-colon results in the errors described above along with a "illegal trailing semi-colon".

For the class def in LEM_AP.h I have
Code:
class LEM_AP {
	friend class SpiderLEM;	// LEM Autopilot Class

public:
	LEM_AP ();
	~LEM_AP ();

	int AP_ProgState; // current autopilot program state
	enum {Off, Killrot, Orient, Hover, Launch, Land} AP_Program; // selected program

	// Autopilot Functions...

for the constructor in LEM_AP.cpp all I have is...
Code:
LEM_AP::LEM_AP () 
{
	LEM_AP::SyncVariables();
}


---------- Post added 08-21-12 at 02:01 AM ---------- Previous post was 08-20-12 at 11:20 PM ----------

So I was looking at various code samples and the way you've described is how Martins did it in the DG but for the ShuttleA he did something different.

He forward declares the "AttitudeReference" class in his ShuttleA.h and then in the ShuttleA.cpp he calls.

Code:
ShuttleA::ShuttleA (OBJHANDLE hObj, int fmodel)
: VESSEL3 (hObj, fmodel)
{
	int i;

	attref = new AttitudeReference (this);
...

It seems a lot simpler than tacking extra ":" items onto the constructor.

Is there a particular advantage or disadvantage to doing it this way?
 

marooder86

Donator
Donator
Joined
Dec 1, 2010
Messages
188
Reaction score
3
Points
33
Location
London
Constructor in SpiderLEM.cpp
Code:
SpiderLEM::SpiderLEM (OBJHANDLE hVessel, int flightmodel)
	: VESSEL3 (hVessel, flightmodel)
	[COLOR="Red"]: autopilot ()[/COLOR]
{
// Set starting values...
This is syntax error. You need to put colon only at the beginig of initialization list(notice there's already one there) and split the instructions with commas
Code:
SpiderLEM::SpiderLEM (OBJHANDLE hVessel, int flightmodel)
	: VESSEL3 (hVessel, flightmodel), autopilot ()
{
// Set starting values...

for the constructor in LEM_AP.cpp all I have is...
Code:
LEM_AP::LEM_AP () 
{
	LEM_AP::SyncVariables();
}
First when you're inside the class scope and the function or a member variable is not static then there's no need to use scope operator(LEM_AP:: ), but that's
just cosmetics.
Secondly, if SyncVariables() does what I think it does(synchronize some autopilot variables with their's SpiderLEM's counterparts), then there's gonna be a problem here, because SpiderLEM's ones won't be existing at this point(AFAIK) and you will have to move that function call after the point in code where SpiderLEM is being created.

So I was looking at various code samples and the way you've described is how Martins did it in the DG but for the ShuttleA he did something different.

He forward declares the "AttitudeReference" class in his ShuttleA.h and then in the ShuttleA.cpp he calls.

Code:
ShuttleA::ShuttleA (OBJHANDLE hObj, int fmodel)
: VESSEL3 (hObj, fmodel)
{
	int i;

	attref = new AttitudeReference (this);
...
That's the second option you can choose(the one with the pointer to object)

It seems a lot simpler than tacking extra ":" items onto the constructor.
Well, it isn't ;), and also you have to trade the colon(which I already told you don't heve to put before every instruction in initiaizaton list) for the "new" operator :).
Is there a particular advantage or disadvantage to doing it this way?
As I've said, you choose one particular option(object or pointer to object) for the particular task/goal you want to acheave and in your case both options
are equally good. The difference will be after compilation process as the first way of doing it(using object as a class member) will result in embeding the
object code and data into dll and the second way(pointer to object) will result in allocating the object dynamically on the heap. Or simply speaking if you
create object in code explicitly
Code:
...
Class_of_objects object_instance;
..
then you get only this one and nothing more

and with pointer to object
Code:
...
Class_of_objects *pointer_to_object;
..
you have the ability to create objects in realtime when you need them and as many as you want if there's enough memory of course.
 

N_Molson

Addon Developer
Addon Developer
Donator
Joined
Mar 5, 2010
Messages
9,295
Reaction score
3,266
Points
203
Location
Toulouse
I think that a simple example, like a ShuttlePB with a class where are put the lift functions, would be invaluable. :2cents:
 

Hlynkacg

Aspiring rocket scientist
Addon Developer
Tutorial Publisher
Donator
Joined
Dec 27, 2010
Messages
1,870
Reaction score
3
Points
0
Location
San Diego
I'm still getting the "SpiderLEM::autopilot' uses undefined class 'LEM_AP'" error despite having it forward declared and including LEM_AP.h

It compiles if I allocate LEM_AP dynamically (use the ShuttleA method) but then I get a CTD whenever I try to acess a "vessel" or "SpiderLEM" function from within LEM_AP.

---------- Post added at 08:56 PM ---------- Previous post was at 08:55 PM ----------

I think that a simple example, like a ShuttlePB with a class where are put the lift functions, would be invaluable. :2cents:

This would be so helpful that there is probably some rule aginst doing it.
 

marooder86

Donator
Donator
Joined
Dec 1, 2010
Messages
188
Reaction score
3
Points
33
Location
London
I'm still getting the "SpiderLEM::autopilot' uses undefined class 'LEM_AP'" error despite having it forward declared and including LEM_AP.h
I'm not sure if it can be done this way, having included header file with class definition and forward declaration of the same class at the same time. They might exlude each other this way and cause this error. But one thing I can tell you for sure, if you have a class with another class object as it's member then you have to go with including the whole member class definition, forward declaration won't work because the compiler won't be able to determine object size.
Basically what Dr Martin said in his post.

It compiles if I allocate LEM_AP dynamically (use the ShuttleA method) but then I get a CTD whenever I try to acess a "vessel" or "SpiderLEM" function from within LEM_AP.
The joy of working with pointers :), probably some access violation. What is crash log saying?
 
Last edited:

Hlynkacg

Aspiring rocket scientist
Addon Developer
Tutorial Publisher
Donator
Joined
Dec 27, 2010
Messages
1,870
Reaction score
3
Points
0
Location
San Diego
obiterlog gives me nothing useful, is there another one I should be looking at?

if I remove the forward declaration I get

Code:
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(90): error C2146: syntax error : missing ';' before identifier 'autopilot'
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(90): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(90): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.cpp(28): error C2065: 'autopilot' : undeclared identifier

on my LEM_AP autopilot line, likewise if I leave the declaration in but comment out "#include "LEM_AP.h" "

i'm starting to think that MartinS' shuttleA style is the way to go, I just need to figure out what an acces violation is and how to avoid it.
 

marooder86

Donator
Donator
Joined
Dec 1, 2010
Messages
188
Reaction score
3
Points
33
Location
London
obiterlog gives me nothing useful, is there another one I should be looking at?

if I remove the forward declaration I get

Code:
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(90): error C2146: syntax error : missing ';' before identifier 'autopilot'
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(90): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(90): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.cpp(28): error C2065: 'autopilot' : undeclared identifier

on my LEM_AP autopilot line, likewise if I leave the declaration in but comment out "#include "LEM_AP.h" "

i'm starting to think that MartinS' shuttleA style is the way to go, I just need to figure out what an acces violation is and how to avoid it.
Are you shure you're including the file the right way cause I see you're using small letters in your files names, so if the LEM_AP header file is actually called "lemap.h " for example then include instruction should be #include "lemap.h" not #include "LEM_AP.h" which I assume is the class name.
:facepalm: Darn I'm stupid:facepalm: this would result in different error. Just scratch what I've said up above.
 
Last edited:

Hlynkacg

Aspiring rocket scientist
Addon Developer
Tutorial Publisher
Donator
Joined
Dec 27, 2010
Messages
1,870
Reaction score
3
Points
0
Location
San Diego
Are you shure you're including the file the right way cause I see you're using small letters in your files names, so if the LEM_AP header file is actually called "lemap.h " for example then include instruction should be #include "lemap.h" not #include "LEM_AP.h" which I assume is the class name.
:facepalm: Darn I'm stupid:facepalm: this would result in different error. Just scratch what I've said up above.

no sweat

LEM_AP header file is actually called LEM_Autopilot.h but I'm a lazy typer. And yes I did check to make sure my caps and spelling were consistant.

as for the rest, I've been reading up on access violations (Yay! Google) and trying to figure out why Martins' AttitudeReference class can call vessel variables without crashing but LEM_AP can't.

I found the following lines in the class defthat I think are signifigant but I'm not sure.

Code:
class AttitudeReference {
public:
	AttitudeReference (const VESSEL *vessel);
[COLOR="Red"]	inline const VESSEL *GetVessel() const { return v; }[/COLOR]

later on in the class there is

Code:
private:
[COLOR="red"]	const VESSEL *v;[/COLOR]

If this does what I think it does it assigns the handle "v" to whatever vessel a particular instance of AttitudeReference is assigned to.

from there functions can be called using the v->

why this would work when vessel-> wouldn't is a mystery to me but whatever. I just want this thing to work so I can get back to the fun stuff. (Flying around in Orbiter)

---------- Post added 08-22-12 at 01:19 AM ---------- Previous post was 08-21-12 at 11:48 PM ----------

sucess :woohoo:

I incorperated the highlighted lines into my code it looks like it works.

---------- Post added at 01:21 AM ---------- Previous post was at 01:19 AM ----------

On the other hand, I've been thinking about writing a tutorial series for budding developers so it'd be nice to if actually I understood what was going on in my own code.

---------- Post added at 01:35 AM ---------- Previous post was at 01:21 AM ----------

ok new problem, sort of.

using the method I descrbed above I can only access functions that are members of the orbiter vessel class it's self.

what do I need to do to get access to SpiderLEM's functions?

---------- Post added at 05:22 AM ---------- Previous post was at 01:35 AM ----------

This is the error I get when I call a function unique to spider LEM from LEM_AP.

Code:
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\lem_autopilots.cpp(269): error C2662: 'SpiderLEM::StageSeparation' : cannot convert 'this' pointer from 'const SpiderLEM' to 'SpiderLEM &'
1>          Conversion loses qualifiers
 

marooder86

Donator
Donator
Joined
Dec 1, 2010
Messages
188
Reaction score
3
Points
33
Location
London
as for the rest, I've been reading up on access violations (Yay! Google) and trying to figure out why Martins' AttitudeReference class can call vessel variables without crashing but LEM_AP can't.
Don't take my words about memory access violation for sure it was a pure guess since this as well as memory leaks are two most common errors while dealing with pointers. Also since I've mentioned memory leaks you did remember about deleting autopilot object didn't you ?:)

I found the following lines in the class defthat I think are signifigant but I'm not sure.

Code:
class AttitudeReference {
public:
	AttitudeReference (const VESSEL *vessel);
[COLOR="Red"]	inline const VESSEL *GetVessel() const { return v; }[/COLOR]

later on in the class there is

Code:
private:
[COLOR="red"]	const VESSEL *v;[/COLOR]

If this does what I think it does it assigns the handle "v" to whatever vessel a particular instance of AttitudeReference is assigned to.
No, you're wrong here, the function GetVessel() doesn't do anything to v pointer, it only provides access to it. Look at it's body, there's only one instuction there "return v;" and no other instructions messing with "v". You should also notice those mysterious "const" specifiers, they are very important. Read the eighth paragraph.

ok new problem, sort of.

using the method I descrbed above I can only access functions that are members of the orbiter vessel class it's self.

what do I need to do to get access to SpiderLEM's functions?
You need to use pointer to SpiderLEM not VESSEL class or use casting since SpiderLEM inherits from VESSEL(shouldn't new addons be inherited from VESSEL3 by the way).

[/COLOR]This is the error I get when I call a function unique to spider LEM from LEM_AP.

Code:
1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\lem_autopilots.cpp(269): error C2662: 'SpiderLEM::StageSeparation' : cannot 'this' pointer from 'const SpiderLEM' to 'SpiderLEM &'
1>          Conversion loses qualifiers
You're violating 'const' protection here as far as I can tell. Your pointer to SpiderLEM is const protected and StageSeparation function doesn't respect that.
 

Hlynkacg

Aspiring rocket scientist
Addon Developer
Tutorial Publisher
Donator
Joined
Dec 27, 2010
Messages
1,870
Reaction score
3
Points
0
Location
San Diego
You need to use pointer to SpiderLEM not VESSEL class or use casting since SpiderLEM inherits from VESSEL(shouldn't new addons be inherited from VESSEL3 by the way).

Figured out that I needed to replace "VESSEL" w/ "SpiderLEM" myself but what is casting?

You're violating 'const' protection here as far as I can tell. Your pointer to SpiderLEM is const protected and StageSeparation function doesn't respect that.

I read the link you provided but I don't see what I violated.

v will always be the same vessel for a given instance of LEM_AP so it should be constant shouldn't it? "StageSeparation()" is a simple void function so I don't see how it would violate constant protection either.
 

marooder86

Donator
Donator
Joined
Dec 1, 2010
Messages
188
Reaction score
3
Points
33
Location
London
...but what is casting?
It's a mechanism/language feature which is being used for explicit type conversions. I bet if you ask google it will provide you with lots of more elaborate explanations, but as you've figured out there's no need for using it here.
v will always be the same vessel for a given instance of LEM_AP so it should be constant shouldn't it?

Well, yes but you have to remember that by placing const specifier before any variable/pointer you tell the compiler this variable cannot be altered, it can only be read and compiler will guard that. I don't want to go deeper into explanation or give any advice as I'm guilty of not using constant correctness mechanism very often in my own code but AFAIK only 'const' functions can work with 'const' variables so if "StageSeparation()" does something with "const SpiderLEM *v;" then it has to be constant too, and you do that by placing 'const' after parenthesis in function declaration.
Code:
type StageSeparation(/*parameters*/) const{}
 

Hlynkacg

Aspiring rocket scientist
Addon Developer
Tutorial Publisher
Donator
Joined
Dec 27, 2010
Messages
1,870
Reaction score
3
Points
0
Location
San Diego
Well I already solved the problem (sort of) by simply making v not a constant.

As N_Molson said, a simple example, like a ShuttlePB with a class where are put the lift functions, would be invaluable.

I'd write it myself but I still have no clue what I'm doing.
 
Top