Programming Forums > Application Development > C++

teencoder · Jan 7th, 2006, 10:40 PM

// enemy and hero class structures
// Bases are to be inherited by their respective class in main file.
// Functions should multiply base values by level to determine health,defense,and attack.
// By: Matthew Henry
// created on 1/7/06 7:04
// last edited on 1/7/06
// v. 1.0 Constructive Criticism phase
// all values must be PUBLIC!!!!
#pragma once
class Saucer
{
public:
	int level;
	int health;
	int defense;
	int attack;
	bool s_alive;
	Saucer()
	{
		attack=3;
		defense=1;
		health=10;
		level=1;
		s_alive=true;
	}
	~Saucer()
	{
	}
	int levelup()
	{
		int level_rec=level;
		level++;
		attack=attack*2;
		defense=defense*2;
		health=health*2;
		if (level==(level_rec+1))
			return 0;
		else
			return 1;
	}
};
class Hero
{
public:
	int level;
	int health;
	int defense;
	int attack;
	bool h_alive;
	Hero()
	{
		attack=5;
		defense=1;
		h_alive=true;
		health=50;
		level=1;
	}
	~Hero()
	{
	}
	int levelup()
	{
		int level_rec=level;
		level++;
		attack=attack*2;
		defense=defense*2;
		health=health*2;
		if (level==(level_rec+1))
			return 0;
		else
			return 1;
	}
};
class Mothership
{
public:
	int level;
	int health;
	int defense;
	int attack;
	bool m_alive;
	Mothership()
	{
		attack=25;
		defense=1;
		health=250;
		level=1;
		m_alive=true;
	}
	~Mothership()
	{
	}
	int levelup()
	{
		int level_rec=level;
		level++;
		attack=attack*2;
		defense=defense*2;
		health=health*2;
		if (level==(level_rec+1))
			return 0;
		else
			return 1;
	}
};

I have a question I'd like to ask is it acceptable to put this in a header file?

Thank you very much I truly appreciate it.

P.S. I used in a header that's why it has #pragma once.

Jessehk · Jan 7th, 2006, 10:55 PM

First of all, it is very bad practise to define functions before you prototype them. Especially when designing classes.

So, taking a simple example,

(Toggle Plain Text)

class T {
        public:
                T() {
                        //do stuff
                }

                void doSomething() {
                        //do stuff
                }
};

//instead, do this

class T {
        public:
                T();
                void doSomething();
};

T::T() {
        //do stuff
}

void T::doSomething() {
        //do stuff
}

And, as I have learned also on this forum, method/function definitions should never be in header files.

Only the prototypes should be included in the header. The definitions (where you say what each function does) should be in a seperate file.

Also, instead of making your properties public, ( which is a fundementally bad idea, becuase I could do something like this in a main() function, which is a very, very bad thing.

(Toggle Plain Text)

Saucer::health = 32003213213123;

) make them private, and provide getproperty methods.
Making variables public instead of making them private tells me your are coding lazily. If you wan't them to be inherited, make them protected, which means that inherited methods will have access to them.

For example,

(Toggle Plain Text)

int getHealth() const {
     return health;
}

Now, C# has something called a property, which I really like, but unfortunately, they aren't available in C++

PS: I hope I don't come off as too critical. Besides what I mentioned, everything looks fine. Keep at it and don't get discouraged.

teencoder · Jan 7th, 2006, 11:38 PM

@Jessehk thanks I will seperate the method definition. I've just starting working with classes seriously.

P.S. methods were not originally in my plans at all I just saw how dandy it would be.

Jessehk · Jan 7th, 2006, 11:42 PM

Quote:

Originally Posted by teencoder

@Jessehk thanks I will seperate the method definition. I've just starting working with classes seriously.

P.S. methods were not originally in my plans at all I just saw how dandy it would be.

No problem.

Can I ask why your class members are all public?
Maybe I can help you work around it.

Master · Jan 7th, 2006, 11:44 PM

why don't it make it easier for yourself by creating just one class and name it class Player. then you can create instances of the class for each player, rather than have repetitive code. then you can just do
Player Saucer;
Player Hero;
Player MotherShip;

teencoder · Jan 7th, 2006, 11:44 PM

Would it be acceptable to link a source file to it with the definitions? Also they have different requirements for the constructer and have slight variations in nameing in some areas I started trying it with inheritance maybe I'll do it next time. Saucer and Mothership are reserved for control by the computer they will eventually be upgraded to hold coordinates, firing info, and AI. This is just getting data management set up before the graphics.

Jason Isom · Jan 7th, 2006, 11:45 PM

Quote:

Originally Posted by Master

why don't it make it easier for yourself by creating just one class and name it class Player. then you can create instances of the class for each player, rather than have repetitive code. then you can just do
Player Saucer;
Player Hero;
Player MotherShip;

Or better yet, create interfaces or use inheritance to move most of the common functionality to base classes.

teencoder · Jan 7th, 2006, 11:51 PM

I wanted to interface with them directly when necessary.

grumpy · Jan 8th, 2006, 12:13 AM

Quote:

Originally Posted by Jessehk

First of all, it is very bad practise to define functions before you prototype them.

At the risk of being pedantic, teencoder did not define functions before he prototyped them. What he did was include the definitions of the function (i.e. the function body) within the class body. So, in this case, the definition is the prototype, not something that came before the prototype.

Quote:

Originally Posted by Jessehk

And, as I have learned also on this forum, method/function definitions should never be in header files.

Only the prototypes should be included in the header. The definitions (where you say what each function does) should be in a seperate file.

My only exception in the above is with the word "never", which you have underlined for emphasis. The reason I take exception is that (well) there are exceptions. In some cases, it is desirable to have inline function definitions, as this allows the compiler to do some gymnastics and generate more efficient code. That means putting function bodies in header files, among other things.

In the process of initially implementing a class, and getting it working right, it is often a good practice to have no implementation of functions in a header file. But, once that is done, it can turn out that judiciously inlining a few functions can help the compiler produce an executable that runs faster. In practice, it often turns out that setter and getter functions (functions that set or retrieve the value of a private variable) are good candidates for inlining, particularly if those functions only do minor error checking.

And, of course, there are template classes or functions: their implementations are inline, more often than not.

Quote:

Originally Posted by Jessehk

Also, instead of making your properties public, ( which is a fundementally bad idea, becuase I could do something like this in a main() function, which is a very, very bad thing.

(Toggle Plain Text)

Saucer::health = 32003213213123;

As a matter of fact, main() could not do this in teencoder's example, as health is a non-static member of his class Saucer. The compiler would complain bitterly if he tried. And that's ignoring the fact that you're using a value that is bigger than can be represented in an int on a lot of compilers.

The type of example you're looking for would be of the form;

(Toggle Plain Text)

  an_instance_of_saucer.health = 123;

Quote:

Originally Posted by Jessehk

) make them private, and provide getproperty methods.
Making variables public instead of making them private tells me your are coding lazily. If you wan't them to be inherited, make them protected, which means that inherited methods will have access to them.

Again, you're making absolute statements of right vs wrong, when there are really shades of grey.

I don't interpret making member variables public rather than private as a sign of laziness. It is, for example, essential if one is calling a function written in C from a C++ compiler.

Your basic principle (make your members private, and then provide public setters and getters) is a good way to achieve encapsulation eg. ensuring that members cannot be changed in invalid ways. But encapsulation is a technique: it is up to the person implementing a class to decide if encapsulation is required.

Quote:

Originally Posted by Jessehk

Now, C# has something called a property, which I really like, but unfortunately, they aren't available in C++

Properties in languages like C# (or in Delphi, and as an extension to C++ offered by Borland's C++ compilers) are really just syntactic sugar. The way properties work is that they effectively mean that a compiler interprets lines like this;

(Toggle Plain Text)

    an_object.SomeProperty = 42;
    value = an_object.SomeProperty;

as;

(Toggle Plain Text)

   an_object.SetSomeProperty(42);
   value = an_object.GetSomeProperty();

While that can be useful in some cases, I'm not really an advocate of such things it can also mean confusion for novices --- and for compiler writers. For example (if C++ supported properties), the following could well be invalid;

(Toggle Plain Text)

   an_object.SomeProperty++;

In practice, some languages that support properties would accept this (or the functional equivalent in that language) and others wouldn't. Why? Because the setter may refuse to allow the new value to be set. This gives the compiler writer (or whoever is specifying the language) the quandary of making this code a compile time error (not allowing the operation at all) or a run time error (allow it to happen, but complain if it isn't allowed). Whichever choice is made (eg a compiler error or a potential run time error), there would be some programmers who want the other choice.

Quote:

Originally Posted by Jessehk

PS: I hope I don't come off as too critical. Besides what I mentioned, everything looks fine. Keep at it and don't get discouraged.

Ah, well. If you came across as being too critical, I just made you look tame.

Basic message: a lot of what you stated are good guidelines but, in some cases, it is necessary to break guidelines, not state them as absolute requirements.

Jessehk · Jan 8th, 2006, 1:00 AM

Quote:

Originally Posted by Grumpy

Basic message: a lot of what you stated are good guidelines but, in some cases, it is necessary to break guidelines, not state them as absolute requirements.

This is true. My apologies (both teencoder and Grumpy(which is a very appropriate name at the present time

)).

Jan 7th, 2006, 10:40 PM	#1
teencoder Hobbyist Programmer Join Date: Jul 2005 Posts: 158 Rep Power: 0	Any Critisisms? (Toggle Plain Text) // enemy and hero class structures // Bases are to be inherited by their respective class in main file. // Functions should multiply base values by level to determine health,defense,and attack. // By: Matthew Henry // created on 1/7/06 7:04 // last edited on 1/7/06 // v. 1.0 Constructive Criticism phase // all values must be PUBLIC!!!! #pragma once class Saucer { public: int level; int health; int defense; int attack; bool s_alive; Saucer() { attack=3; defense=1; health=10; level=1; s_alive=true; } ~Saucer() { } int levelup() { int level_rec=level; level++; attack=attack2; defense=defense2; health=health2; if (level==(level_rec+1)) return 0; else return 1; } }; class Hero { public: int level; int health; int defense; int attack; bool h_alive; Hero() { attack=5; defense=1; h_alive=true; health=50; level=1; } ~Hero() { } int levelup() { int level_rec=level; level++; attack=attack2; defense=defense2; health=health2; if (level==(level_rec+1)) return 0; else return 1; } }; class Mothership { public: int level; int health; int defense; int attack; bool m_alive; Mothership() { attack=25; defense=1; health=250; level=1; m_alive=true; } ~Mothership() { } int levelup() { int level_rec=level; level++; attack=attack2; defense=defense2; health=health2; if (level==(level_rec+1)) return 0; else return 1; } }; // enemy and hero class structures // Bases are to be inherited by their respective class in main file. // Functions should multiply base values by level to determine health,defense,and attack. // By: Matthew Henry // created on 1/7/06 7:04 // last edited on 1/7/06 // v. 1.0 Constructive Criticism phase // all values must be PUBLIC!!!! #pragma once class Saucer { public: int level; int health; int defense; int attack; bool s_alive; Saucer() { attack=3; defense=1; health=10; level=1; s_alive=true; } ~Saucer() { } int levelup() { int level_rec=level; level++; attack=attack2; defense=defense2; health=health2; if (level==(level_rec+1)) return 0; else return 1; } }; class Hero { public: int level; int health; int defense; int attack; bool h_alive; Hero() { attack=5; defense=1; h_alive=true; health=50; level=1; } ~Hero() { } int levelup() { int level_rec=level; level++; attack=attack2; defense=defense2; health=health2; if (level==(level_rec+1)) return 0; else return 1; } }; class Mothership { public: int level; int health; int defense; int attack; bool m_alive; Mothership() { attack=25; defense=1; health=250; level=1; m_alive=true; } ~Mothership() { } int levelup() { int level_rec=level; level++; attack=attack2; defense=defense2; health=health2; if (level==(level_rec+1)) return 0; else return 1; } }; I have a question I'd like to ask is it acceptable to put this in a header file? Thank you very much I truly appreciate it. P.S. I used in a header that's why it has #pragma once. __________________ Geeks may not be cool now but in the long run they prosper. Last edited by teencoder; Jan 7th, 2006 at 10:51 PM.

Jan 7th, 2006, 10:55 PM	#2
Jessehk The Oblivious One Join Date: May 2005 Location: Ontario, Canada Posts: 648 Rep Power: 4	First of all, it is very bad practise to define functions before you prototype them. Especially when designing classes. So, taking a simple example, (Toggle Plain Text) class T { public: T() { //do stuff } void doSomething() { //do stuff } }; //instead, do this class T { public: T(); void doSomething(); }; T::T() { //do stuff } void T::doSomething() { //do stuff } class T { public: T() { //do stuff } void doSomething() { //do stuff } }; //instead, do this class T { public: T(); void doSomething(); }; T::T() { //do stuff } void T::doSomething() { //do stuff } And, as I have learned also on this forum, method/function definitions should never be in header files. Only the prototypes should be included in the header. The definitions (where you say what each function does) should be in a seperate file. Also, instead of making your properties public, ( which is a fundementally bad idea, becuase I could do something like this in a main() function, which is a very, very bad thing. (Toggle Plain Text) Saucer::health = 32003213213123; Saucer::health = 32003213213123; ) make them private, and provide getproperty methods. Making variables public instead of making them private tells me your are coding lazily. If you wan't them to be inherited, make them protected, which means that inherited methods will have access to them. For example, (Toggle Plain Text) int getHealth() const { return health; } int getHealth() const { return health; } Now, C# has something called a property, which I really like, but unfortunately, they aren't available in C++ PS: I hope I don't come off as too critical. Besides what I mentioned, everything looks fine. Keep at it and don't get discouraged. __________________ Dr. Zoidberg: [ecstatic] I'm going to a movie... with FRIENDS! Last edited by Jessehk; Jan 7th, 2006 at 11:09 PM.

Jan 7th, 2006, 11:38 PM	#3
teencoder Hobbyist Programmer Join Date: Jul 2005 Posts: 158 Rep Power: 0	@Jessehk thanks I will seperate the method definition. I've just starting working with classes seriously. P.S. methods were not originally in my plans at all I just saw how dandy it would be. __________________ Geeks may not be cool now but in the long run they prosper.

Jan 7th, 2006, 11:44 PM	#6
teencoder Hobbyist Programmer Join Date: Jul 2005 Posts: 158 Rep Power: 0	Would it be acceptable to link a source file to it with the definitions? Also they have different requirements for the constructer and have slight variations in nameing in some areas I started trying it with inheritance maybe I'll do it next time. Saucer and Mothership are reserved for control by the computer they will eventually be upgraded to hold coordinates, firing info, and AI. This is just getting data management set up before the graphics. __________________ Geeks may not be cool now but in the long run they prosper. Last edited by teencoder; Jan 7th, 2006 at 11:55 PM.

Jan 7th, 2006, 11:51 PM	#8
teencoder Hobbyist Programmer Join Date: Jul 2005 Posts: 158 Rep Power: 0	I wanted to interface with them directly when necessary. __________________ Geeks may not be cool now but in the long run they prosper.

Jan 7th, 2006, 11:44 PM	#5
Master Programmer Join Date: Sep 2005 Location: GA Posts: 99 Rep Power: 4	why don't it make it easier for yourself by creating just one class and name it class Player. then you can create instances of the class for each player, rather than have repetitive code. then you can just do Player Saucer; Player Hero; Player MotherShip;

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Programming Forums > Application Development > C++

Any Critisisms?