# Simple Inheritance Question

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

## Recommended Posts

This is my first post on this forum, so please be patient with me...:D I got this working code: /* ctask.h */
 class ctask
{
public:
virtual void DoSomething();
};


/* ccomp.h */
 class ccomp: public ctask
{
public:
ccomp();
~ccomp();
void DoSomething();
/*... other methods ...*/
};

void ccomp::DoSomething()
{
/* do something..*/
}


/* ckernel.h */
 #include <ctask.h>
#include <ccomp.h>

class ckernel
{
public:

ckernel();
~ckernel();
void Run();
}

ckernel::ckernel()
{
}

ckernel::~ckernel()
{
{
}
}

void ckernel::Run()
{
{
}
}

{
{
}
else
{
/* do nothing */
}
}



##### Share on other sites
The solution to your first issue is using include guards. For example:
/// This goes in ctask.h:#ifndef _CTASK_H_#define _CTASK_H_// The rest of the header's contents goes here#endif

Note that the name name you define, _CTASK_H_ in the example, is not really important as long as it is unique to that header file.

As for the rest of your question, please edit your original message and put the code inside source or code tags. This will make your code much more readable.

##### Share on other sites
Quote:
 Original post by toro3. What do you think about the way this "ckernel" class is implemented, do you have any suggestions?

Since we're coding in c plus plus, I'd recommend storing your tasks in a vector or list.
Quote:
 2. "(ctask *) new ccomp" is acceptable?

I would do ctask( ccomp ). (create the task beforehand, then pass a reference to the kernel)
Quote:
 4. The coding style is acceptable?

Besides the above, implementation seems OK. As for the format, I can't tell. You can use the source tags described here. Besides that, you may want to look at these articles. They describe a Kernel system similar to yours.

##### Share on other sites

Peregrin wrote:
Quote:
 The solution to your first issue is using include guards

I do know the mechanism, however I didn't know that it is called guards... my question is if it is common to C++ implementations or there is any reason why I should avoid it?

stylin wrote:
Quote:
 Since we're coding in c plus plus, I'd recommend storing your tasks in a vector or list.

probably the code is not complete, which means that I would like to add something like this:
/*ccomp2.h*/
class ccomp2: public ctask {     public:     ccomp2();     ~ccomp2();          void DoSomething();     /*... other methods than ccomp class...*/ }; void ccomp2::DoSomething() {     /* do something..*/ }

so in ckernel.h I would update the cosntructor with this:
 ckernel::ckernel() {      AddTask((ctask *) new ccomp);      AddTask((ctask *) new ccomp2); }

In this scenario, is still possible to use vector class?

stylin wrote:
Quote:
 I would do ctask( ccomp ). (create the task beforehand, then pass a reference to the kernel)

Sorry, I'm not familiar with this, but I will look into it. Thanks for pointing out that article.

[Edited by - toro on September 8, 2005 6:10:59 AM]

##### Share on other sites
Quote:

The #ifdef solution is fine. Since inclusion guards protect multiple inclusion of a file, it should be the filename that is part of the defined name, not just a class in this file.
Quote:
 2. "(ctask *) new ccomp" is acceptable?

Why not just "new ccomp" without a cast ? The compiler will downcast automatically.
Quote:
 3. What do you think about the way this "ckernel" class is implemented, do you have any suggestions?

Maybe you should not implement the class in the header, but in another file, i.e. ckernel.cc
Usually only template classes or very small functions of a class are implemented in the header. If you implement small member functions in the class, they are inline by default, which may even reduce code size since there is no need to push the parameters on the stack and call the functions.
Another thing are the #include directives. For header files that are part of your project, you should use #include "ctask.h", for header files of the (standard) library, you should use #include <iostream>.
Also define a constant if you use it multiple times.
Example:
class ckernel{        enum { MAX_TASK_COUNT = 21 };    public:        ctask* pctask[MAX_TASK_COUNT];};

Quote:
 4. The coding style is acceptable?

Having a style is a good thing. It makes your program easier to read, even if others are not familar with your style. Also comments are a great help. Check out tools like doxygen, that allow you to create a browseable documentation for your class hierarchy. Also grouping statements into paragraphs (i.e. three statements that belong together, then an empty line) allows to follow the code more easily.
Also some spaces would makes things easier to read:
for (int i=0; i<task_count;i++)

becomes
for (int i = 0; i < task_count; i++)

When using a type, write all of what belongs to the type together:
ctask *pctask[21];

becomes
ctask* pctask[21];

##### Share on other sites
Quote:
 Original post by toro I do know the mechanism, however I didn't know that it is called guards... my question is if it is common to C++ implementations or there is any reason why I should avoid it?

Not at all. It's the right thing to do. (OPS: plus pluses are getting chopped)

Quote:
  ctask *pctask[21]; int task_count;

You're coding in C plus plus now, time to drop those unsafe, pain in the butt C arrays[smile]. Also, you're limiting your task count - why?

Quote:
 void ckernel::Run() { for (int i=0; iDoSomething(); } }

Here you're iterating through your task array, and executing them all. Then what? What if a task wants to die? What if there are no more tasks? Currently you have no support for killing tasks without alot of hassle - this is why you use a std::list. Instead of having seperate functions in your kernel, you can do all of this in this function.

As long as you have living tasks, iterate through your task list, and DoSomething() to all tasks that are not ready to die. Iterate again and remove those tasks that are (if you're using smart pointers this effectively destroys the task for you).

So, ckernel::Run executes until there are no more tasks. (What else would you want to do inbetween your task loop? Remember, your tasks are the objects that make up your system, the kernel just sees that they get executed.)

style-wise, I can read your code well. And if you have nobody else that's looking at your code, (or going to be looking at it) then all that matters is if you can read it. I do the following:
class SomeClass {   int some_int_;   int * psome_int_;public:   SomeClass( int some_var ):      some_int_(some_var), psome_int_( new int( some_int_ )) {};  ~SomeClass() { delete psome_int_; };};

##### Share on other sites
I'm not sure why no-one has mentioned this, but the
else{     /* do nothing */}

is entirely pointless. In fact I'm pretty sure you can't put an else after a for loop.

##### Share on other sites
RAZORUNREAL wrote:
Quote:
 I'm not sure why no-one has mentioned this, but theelse{ /* do nothing */}is entirely pointless. In fact I'm pretty sure you can't put an else after a for loop.

You are right about the else {} after for...it was a typo.(corrected)

I put the else {/* do nothing */} because if anybody looks in my code, he will know that I didn't ignore the else branch and it can be sure that nothing should be executed on that branch. But other than this is pretty pointless...

##### Share on other sites
nmi, stylin thanks for ur great replies, however I need some time to "assimilate" this new knowledge...:D

nmi wrote:
Quote:
 Usually only template classes or very small functions of a class are implemented in the header...

You suggest to implement classes inside .c files or to split the definition of a class between a .c file and a .h file?
Like this:

nmi wrote:
Quote:
 enum { MAX_TASK_COUNT = 21 };

is this better practice than #define?

stylin wrote:
Quote:
 You're coding in C plus plus now, time to drop those unsafe, pain in the butt C arrays. Also, you're limiting your task count - why?

Quote:
 So, ckernel::Run executes until there are no more tasks. (What else would you want to do inbetween your task loop? Remember, your tasks are the objects that make up your system, the kernel just sees that they get executed.)

I intend that every task will be a component of an game engine: render, audio, ai or any other "independent" component. So these tasks cannot die and there shouldn't be other dinamic tasks to be added to the task list after the compile time and they should respect a certain order of execution. That's what I try to do, I don't know if this is the best aproach to a task scheduler but probably I will find out soon. This is the reason why I didn't tried to use <vector> or <list> containers, which I assume that they can provide a more generic implementation. I also want to implement a scheduler based on time bases, therefore ctask class can contain additional data needed for scheduler.
I will look into the smart pointer matter, but I'm really undecided if I should go for a generic implementation like you suggest or remain with a C-like implementation.

##### Share on other sites
Quote:
 Original post by toroYou suggest to implement classes inside .c files or to split the definition of a class between a .c file and a .h file?

Put the class definition:
class X{	public:		X();		int memberFunction():	private:		int variable_;};

in a header, with inclusion guards (as shown in the link you posted) and the class member definitions:
X::X(){	// code}

in a .cpp or .cc file (not a .c, since many C++ compilers are also C compilers and unless told otherwise use the file extension to determine if the file is C++ or C code).
Quote:
nmi wrote:
Quote:
 enum { MAX_TASK_COUNT = 21 };

is this better practice than #define?

That or even just unsigned int const MAX_TASK_COUNT = 21 (probably in the anonymous namespace to keep it local to the translation unit) are much better than #define because they are part of the language. #define is part of the preprocessor and is simply text replaced. It therefore ignores scoping, type and context and can result in some nasty errors, i.e.:
// lots of lines of code// in some header file that gets included#define COUNT 7;// lots of lines of codevoid somefunction(){	int const COUNT = numberOfWidgets;}

That's what you see. After the preprocessor has done its work what the compiler sees is:
// lots of lines of codevoid somefunction(){	int const 7 = numberOfWidgets;}

which is obviously nonsensical.

Quote:
 I intend that every task will be a component of an game engine: render, audio, ai or any other "independent" component. So these tasks cannot die and there shouldn't be other dinamic tasks to be added to the task list after the compile time and they should respect a certain order of execution. That's what I try to do, I don't know if this is the best aproach to a task scheduler but probably I will find out soon. This is the reason why I didn't tried to use or containers, which I assume that they can provide a more generic implementation. I also want to implement a scheduler based on time bases, therefore ctask class can contain additional data needed for scheduler. I will look into the smart pointer matter, but I'm really undecided if I should go for a generic implementation like you suggest or remain with a C-like implementation.

For your pctask array a raw stack based array may be appropriate if you know exactly how many tasks there will be. If not, use a SC++L container. As for pointers, it depends. Do you enjoy spending hours in front of the debugger trying to find out where your pointers are being double deleted or leaked or trying to find the cause of an intermittent crash that turns out to be caused by using a pointer after it was deleted? If so then stick with a C-like implementation. If not then grab the boost smart pointer library, use smart pointers and become much more productive.

Enigma

• ### What is your GameDev Story?

In 2019 we are celebrating 20 years of GameDev.net! Share your GameDev Story with us.

• 13
• 9
• 9
• 15
• 14
• ### Forum Statistics

• Total Topics
634070
• Total Posts
3015332
×