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

[Hlynkacg]

Like the title says, I need help breaking a monster-sized (vessel) class into more managable sub-classes.

Specifically I want to take my Lunar Lander's autopilot functions/variables and (for the sake of readability) place them in their own class.

However the base vessel class still needs acces to some of those variables for situations like.

if (ThrotControlAuto()) SetThrusterGroupLevel(THGROUP_HOVER, AP_CommandedThrust);

I tried copying the syntax of the DG's AAP but that hasn't worked

As per the DG example LEM_AP is defined as a friend class to SpiderLEM (my vessel class)

class SpiderLEM: public VESSEL3 {
	friend class LEM_AP;	// LEM Autopilot Class

public:
	SpiderLEM (OBJHANDLE hVessel, int flightmodel);
	~SpiderLEM ();
...

The LEM_AP class itself looks like this

// ==============================================================
//		ORBITER MODULE: 'SPIDER' LUNAR EXCURSION MODULE
//			A custom Vessel For Orbiter 2010/2011
//  
// LEM_Autopilots.h
// Class interface for Lunar Excursion Module Autopilot
// ==============================================================

#include "Orbitersdk.h"
#include "SpiderLEM.h"

class VESSEL3;

class LEM_AP: public SpiderLEM {
public:
	LEM_AP (VESSEL3 *v);
	virtual ~LEM_AP ();

	// Autopilot Functions
	void	OrientForBurn(VECTOR3& tgtVector);
	void	SetPitch(double tgtPitch);
	void	SetRoll(double tgtRoll);
	void	SetHdg(double tgtHdg);
	void	AutoHover(void);
	void	AutoAscent(double simt);
	void	AutoLand(double simt);

	double	rRate;
	double	AP_CommandedThrust, AP_CommandedYaw, AP_CommandedRoll, AP_CommandedPitch; // Autopilot control inputs

protected:
	VESSEL3 *vessel;
	
private:
	int		AP_ProgState; // current autopilot program state
};

Finally we have the .cpp file which looks like this...

#include "LEM_Autopilots.h"

LEM_AP::LEM_AP (SpiderLEM *vessel)
{
}

LEM_AP::~LEM_AP ()
{
}
Assorted Functions beyond this point...

As it stands I'm getting a compiler error wich states that the LEM_AP constructor is invalid.

If I comment the constructor out the code compiles but I am unable to call any of LEM_AP's functions.

Please help a C++ noob figure this out.

 

[orb]

class LEM_AP: public SpiderLEM {
public:
	LEM_AP (VESSEL3 *v);

LEM_AP::LEM_AP (SpiderLEM *vessel)
{
}

As it stands I'm getting a compiler error wich states that the LEM_AP constructor is invalid.

Did you try this way instead?:

LEM_AP::LEM_AP (VESSEL3 *vessel) : SpiderLEM (vessel) {
}

EDIT: It won't work though, because SpiderLEM's constructor is different:

SpiderLEM (OBJHANDLE hVessel, int flightmodel);

You need an OBJHANDLE of the vessel and flghtmodel to be passed to the SpiderLEM's constructor.

Are you sure you want to extend the SpiderLEM class with LEM_AP or only make it a friend class?

 

[Hlynkacg]

Did you try this way instead?:

LEM_AP::LEM_AP (VESSEL3 *vessel) : SpiderLEM (vessel) {
}

EDIT: It won't work though, because SpiderLEM's constructor is different:

SpiderLEM (OBJHANDLE hVessel, int flightmodel);

I did and as you said, it didn't work.

You need an OBJHANDLE of the vessel and flghtmodel to be passed to the SpiderLEM's constructor.

How would I go about doing this?

Are you sure you want to extend the SpiderLEM class with LEM_AP or only make it a friend class?

No, I'm not sure.

My problem is simply that the class definition for SpiderLEM itself is rapidly becoming unmanageably large. I thought that breaking various sub-systems off into thier own sub-classes would help counter this.

 

[orb]

When you extend SpiderLEM class, which extends VESSEL3 class, you basically create another vessel class based on SpiderLEM, so another vessel. If LEM_AP is just SpiderLEM's autopilot, it shouldn't extend SpiderLEM. Remove ": public SpiderLEM" from the class declaration.

 

[Hlynkacg]

When you extend SpiderLEM class, which extends VESSEL3 class, you basically create another vessel class based on SpiderLEM, so another vessel. If LEM_AP is just SpiderLEM's autopilot, it shouldn't extend SpiderLEM. Remove ": public SpiderLEM" from the class declaration.

So the class declaration should look like...

class LEM_AP {
public:
	LEM_AP (VESSEL3 *v);
	virtual ~LEM_AP ();

	// Autopilot Functions

?

 

[orb]

So the class declaration should look like...

Yes. If you need to access protected members of SpiderLEM, you want to add "friend class SpiderLEM;" to its declaration, too.

 

[Hlynkacg]

Ok,

made the change and added SpiderLEM (the primary vessel class) as a friend of LEM_AP.

Now the compiler error I'm geting is lots of undeclared identifiers.

Functions with the "SpiderLEM::" prefix are marked "illegal reference to non-static member"

Do I tell it to get those values from SpiderLEM?

Am I going to have to redeclare them?

 

[orb]

Functions with the "SpiderLEM::" prefix are marked "illegal reference to non-static member"

You don't reference non-static members with:

SpiderLEM::member

but with the variable holding pointer to instance of the SpiderLEM class:

vessel->member

 

[Hlynkacg]

What if the variable or function in question is unique to SpiderLEM (not a member of the VESSEL3 class)?

Likewise How do I call LEM_AP functions from within Spider?

 

[marooder86]

Likewise How do I call LEM_AP functions from within Spider?

Depends on how you create the autopilot in lander class. If it something like

class Lander
{
...
LEM_AP autopilot;
};

then you call it's functions like that

//somewhere in code
...
autopilot.some_autopilot_function()
...

If you ceate your autopilot dynamically then you call it's functions with pointer that points to it.

class Lender
{
...
LEM_AP *autopilot;
};

//somewhere in code
...
autopilot->some_autopilot_function()
...

And by the way, if you don't use virtual functions in your autopilot class then it's destructor doesn't have to be virtual, the ordinary one will do just fine.
One more thing is if you haven't corrected it already. You've declared such constructor in your autopilot class

LEM_AP (VESSEL3 *v);

and you're trying to implement it this way

LEM_AP::LEM_AP (SpiderLEM *vessel)
{
}

and those are two completely diffrent constructors, you have to implement what you've declared

LEM_AP::LEM_AP (VESSEL3 *vessel)
{
}

 

[orb]

What if the variable or function in question is unique to SpiderLEM (not a member of the VESSEL3 class)?

Is `vessel` always holding an instance of SpiderLEM class? If so, you should have declared it as `SpiderLEM * vessel`. If it can hold instance of other vessel class based on VESSEL3, you can do `((SpiderLEM *)vessel)->member` after checking if the `vessel` is of `SpiderLEM` class.

 

[Hlynkacg]

Depends on how you create the autopilot in lander class. If it something like

class Lander
{
...
LEM_AP autopilot;
};

then you call it's functions like that

//somewhere in code
...
autopilot.some_autopilot_function()
...

If you ceate your autopilot dynamically then you call it's functions with pointer that points to it.

class Lender
{
...
LEM_AP *autopilot;
};

//somewhere in code
...
autopilot->some_autopilot_function()
...

Are there advantages to doing it one way over the other?

Is `vessel` always holding an instance of SpiderLEM class? If so, you should have declared it as `SpiderLEM * vessel`. If it can hold instance of other vessel class based on VESSEL3, you can do `((SpiderLEM *)vessel)->member` after checking if the `vessel` is of `SpiderLEM` class.

At the moment it will always be an instance of "SpiderLEM" but I do have plans to expand in that I eventually want to develope a whole family of "Spiders" that share a common functionality.

Uderstand that aside from having a pretty strong math background I'm coming into this armed with little more than the Orbiter API documentation, and a C++ for beginners turorial.

I've gotten as far as I have by copying/modifying the SDK samples and through help from this very forum.

---------- Post added at 01:23 AM ---------- Previous post was at 12:26 AM ----------

Another quick question...

if I add a clbkPreStep or clbkPostStep to my LEM_AP class will orbiter call it?

it'd be nice from a code readability stand point if I could move AP's "house keeping" along with the AP.

 

[orb]

At the moment it will always be an instance of "SpiderLEM" but I do have plans to expand in that I eventually want to develope a whole family of "Spiders" that share a common functionality.

As long as all of them will extend some common class, you can declare the `vessel` variable to point to that class, be it a `SoiderLEM` or a `Spider` class (but then SpiderLEM will need to extend the Spider class extending the VESSEL3 class, and not just VESSEL3 class).

if I add a clbkPreStep or clbkPostStep to my LEM_AP class will orbiter call it?

They won't be called. LEM_AP class isn't registered in Orbiter to receive such calls. It's your own class. You need to call autopilot functions which must be called in each step from the vessel's clbkPreStep or clbkPostStep.

 

[marooder86]

Are there advantages to doing it one way over the other?

It's not the matter of adventage of one option over the other as it'a matter of program design you've choosed. There are situations when storing object of one class as a member of another one are desirable and logical solution, and there are times when pointer option is better or you simply have no other way of implementing it. For example when the object of a class(let's call it classB) which is a member of another class(classA) does not exist at the point of program execution where you're creating classA object then you have to use pointer solution and wait for classB object to be created so you can get the pointer to it.
From my point of view, in your case both ways would be equally good, although personally I would go with the first option(storing the actual object as a member) since both classes are strongly interconnected and depand on each other existance.
But you know, that's only my :2cents:.

 

[Hlynkacg]

I'm probably missing something obvious so bear with me...

I added to

"class LM_AP Autopilot;"

to my SpiderLEM class but the compiler is flagging it as undeclared.

do I have to include my LEM_Autopilot.h file in SpiderLEM.h? (or move the class declaration to SpiderLEM.h?)

If so wouldn't this cause issues with recursion? (LEM_Autopilot.h includes SpiderLEM.h)

 

[Urwumpe]

do I have to include my LEM_Autopilot.h file in SpiderLEM.h? (or move the class declaration to SpiderLEM.h?)

That solution - both of them are valid, the first is more useful for finding things again without relying too much on the IDE.

If so wouldn't this cause issues with recursion? (LEM_Autopilot.h includes SpiderLEM.h)

No, because as good C++ programmer, you write

#pragma once

at the beginning of every .h file to make sure it is only parsed once by the compiler (for every .cpp file, since every .cpp file is a single compiler run).

 

[martins]

If a class A contains a pointer to another class B, then the full class interface for B is not required, but at the time of declaring A it must at least be known that class B exists. This can be done with a forward declaration. This way, A and B can refer to each other without creating a circular dependency.

class B;  // forward declaration of B

class A {  // declaration of A
   B *pb;  // ok
   B b;  // error, because interface of B is not yet known
};

class B { // declaration of B
  A *pa;  // ok
  A a;   // ok
};

If you include an actual instance of B rather than a pointer, then the full interface of B must be known at that point.

 

[marooder86]

I'm probably missing something obvious so bear with me...

I added to

"class LM_AP Autopilot;"

to my SpiderLEM class but the compiler is flagging it as undeclared.

Well it might be my rusty english plus my little bit entangled explanation that makes it even more complicated then it really is.
Here in your SpiderLEM class declaration there is no need to repeat that keyword "class" because making a class object a member of another class is nothing more then something like this

//in SpiderLEM header file
...
#include "LM_AP.h" // you declare that class and all that's within it in it's own
                          //   header file, that's one of basics and common rules for 
                          //  C++ : "one class two files"
class SpiderLEM
{
  ...
  // some other class functions and members
  ...
  LM_AP autopilot; // that's it, that's all you have to do tell the compiler that 
                         //    autopilot is a member of a class SpiderLEM and it is 
                         //   an object of class LM_AP                 
};

do I have to include my LEM_Autopilot.h file in SpiderLEM.h? (or move the class declaration to SpiderLEM.h?)

As Urwumpe said both solutions will work, but as for

move the class declaration to SpiderLEM.h?

If you mean moving LM_AP declaration into SpiderLEM.h file then remeber the rule: "one class two files" and don't do it.

 

[Hlynkacg]

Well I've included LEM_AP's .h file in SpiderLEM.h but i'm still getting the following errors

1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(78): error C2079: 'SpiderLEM::autopilot' uses undefined class 'LEM_AP'

and

1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.cpp(15): error C2512: 'LEM_AP' : no appropriate default constructor available

 

[marooder86]

Well I've included LEM_AP's .h file in SpiderLEM.h but i'm still getting the following errors

1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.h(78): error C2079: 'SpiderLEM::autopilot' uses undefined class 'LEM_AP'

and

1>c:\users\hlynkacg\documents\visual studio 2010\projects\spiderlem\spiderlem.cpp(15): error C2512: 'LEM_AP' : no appropriate default constructor available

I'm not sure about first error(show that particular code line) 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.

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