Problem with simple class

Started by
7 comments, last by Zakwayda 15 years, 8 months ago
I am reading about classes and I understood all the examples so I thought I would play around with them and try and create a small program using classes. I was trying some functions that one might find in a blackjack game. I wrote a simple blackjack game using only functions and it was a great way to learn, but already I have a long list of errors and my code is very tiny.
#include <iostream>
#include <ctime>
#include <cstdlib>
using namespace std;

class Player
{
public:
	int GetCard();
private:
}

int Player::GetCard()
{
	int card;
	card = (rand()%11)+1;
	return card;
}


int main()
{
	srand(time(0));

	Player Human;
	Human.GetCard();
	
	cout << "Your first card was: " << Human.GetCard();

	return 0;
}
Here is the long list of errors and warnings I am getting. c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(28): error C2264: 'Player::GetCard' : error in function definition or declaration; function not called c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(26): error C2264: 'Player::GetCard' : error in function definition or declaration; function not called c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(23): warning C4244: 'argument' : conversion from 'time_t' to 'unsigned int', possible loss of data c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(14): error C2371: 'Player::GetCard' : redefinition; different basic types c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(14): error C2556: 'Player Player::GetCard(void)' : overloaded function differs only by return type from 'int Player::GetCard(void)' c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(13): error C2628: 'Player' followed by 'int' is illegal (did you forget a ';'?) Im sure I am not using something right, but I am not sure what. Any help would be appreciated. Thanks
Advertisement
Quote:Original post by Bernini1
I am reading about classes and I understood all the examples so I thought I would play around with them and try and create a small program using classes. I was trying some functions that one might find in a blackjack game. I wrote a simple blackjack game using only functions and it was a great way to learn, but already I have a long list of errors and my code is very tiny.

#include <iostream>#include <ctime>#include <cstdlib>using namespace std;class Player{public:	int GetCard();private:}; /* NOTE: added this */int Player::GetCard(){	int card;	card = (rand()%11)+1;	return card;}int main(){	srand(time(0));	Player Human;	Human.GetCard();		cout << "Your first card was: " << Human.GetCard();	return 0;}


Here is the long list of errors and warnings I am getting.
c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(28): error C2264: 'Player::GetCard' : error in function definition or declaration; function not called
c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(26): error C2264: 'Player::GetCard' : error in function definition or declaration; function not called
c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(23): warning C4244: 'argument' : conversion from 'time_t' to 'unsigned int', possible loss of data
c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(14): error C2371: 'Player::GetCard' : redefinition; different basic types
c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(14): error C2556: 'Player Player::GetCard(void)' : overloaded function differs only by return type from 'int Player::GetCard(void)'
c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(13): error C2628: 'Player' followed by 'int' is illegal (did you forget a ';'?)

Im sure I am not using something right, but I am not sure what. Any help would be appreciated.

Thanks


You just forgot the semi-colon at the end of the class definition. The above code should compile error-free.
Oh man do I feel dumb. :( I kept clicking the error messages and it pointed me to unrelated areas of my code which really threw me off.

Thanks for the help, man.
Quote:Oh man do I feel dumb. :( I kept clicking the error messages and it pointed me to unrelated areas of my code which really threw me off.


Quote:
c:\Documents and Settings\Chad\My Documents\Visual Studio Projects\Blackjack_ClassPrac1\main.cpp(13): error C2628: 'Player' followed by 'int' is illegal (did you forget a ';'?)


Er, well, that's where the problem is - it's the definition of the class, followed by the 'int' intended return type for that function body - and it even suggests what might be wrong, so...
Now that your problem's fixed, there are some improvements that could be made to your code.

1) I/many don't like using namespace std in such a wide scope; keep it in the most limited scope you can - which frequently means don't use it at all.

2) Don't bother using scope modifiers, like private, if they're not really doing anything.

3) You don't need to instantiate, initialise and return an int on separate lines, you can combine them.

4) With point 3 in mind, you end up with just a one-liner and so it's a good candidate to be placed inline.

5) You invoke Player::GetCard but to no avail because you don't store the value and it may not be optimised away either because of rand.

6) return 0 at the end of main is implicit, you don't have to put it yourself.

And so:

#include <iostream>#include <ctime>#include <cstdlib>class Player{public:    int GetCard() { return (rand()%11)+1; }};int main(){	srand(time(0));	Player Human; // Even this could go, but I'll leave it.	std::cout << "Your first card was: " << Human.GetCard();}
Personally, I think using the std namespace, assuming your code calls on std frequently, is perfectly fine.

Not using it just creates unnecessarily verbose code which doesn't read as well. :-/
In the end though, It's really just a matter of what you feel more comfortable with.
Quote:Original post by brent_w
In the end though, It's really just a matter of what you feel more comfortable with.
Unfortunately it's a little more involved than that.
Essentially everything living in the std namespace is promoted to a wider namespace - There is nothing stopping you from adding it a-top some header that then gets included throughout a large code base; this would cause what is known as 'namespace pollution'. The rammification of this is the increased chance of name collisions for which your compiler may be unable to disambiguate between, so your code breaks.

We tend to want to avoid namespace pollution as much as possible to avoid this and so you apply using namespace std in the most restrictive scope you can.
For a concrete example of how a using directive can bite you in the rear, consider this example:
#include <d3dx9.h>#include <iostream>using namespace std;void rotate(D3DMATRIX * in_a, D3DMATRIX * in_b, D3DMATRIX * out) {  cout << "in my rotate" << std::endl;}int main(int, char **) {  D3DXMATRIX a, b, out;    rotate(&a, &b, &out);  return 0;}

When you run this, you'll get the output "in my rotate", as you might expect. (If you aren't familiar with Direct3D, this works because D3DXMATRIX is derived from D3DMATRIX.) However, if you change the file very slightly:
#include <d3dx9.h>#include <iostream>#include <algorithm>using namespace std;void rotate(D3DMATRIX * in_a, D3DMATRIX * in_b, D3DMATRIX * out) {  cout << "in my_rotate" << std::endl;}int main(int, char **) {  D3DXMATRIX a, b, out;    rotate(&a, &b, &out);  return 0;}

You'll find that it no longer prints the expected output. The reason is that the algorithm header includes a template function std::rotate() that will accept three pointers. Since the ::rotate() function takes three D3DMATRIX * arguments and not three D3DXMATRIX * arguments, the template function will be considered to be a better match and will end up calling std::rotate() instead.
Quote:Original post by brent_w
Personally, I think using the std namespace, assuming your code calls on std frequently, is perfectly fine.

Not using it just creates unnecessarily verbose code which doesn't read as well. :-/
In the end though, It's really just a matter of what you feel more comfortable with.
IMO, this isn't really just a 'six of one, half-dozen of the other' issue; a strong argument can be made that using 'using' directives is rarely the right thing to do.

Now, I assume you're not saying it's ok to use 'using' directives in header files, right? As for source files, that's up to you, more or less, but as previously noted it can end up causing problems.

If you're really opposed to having to type std occasionally (which, by the way, many folks find makes code more clear, not less), here are a couple of options:

1. Use typedefs, e.g.:
typedef std::vector<Foo> foos_t;
2. Use 'using' declarations rather than directives:
using std::cout;using std::endl;using std::string;using std::vector;

This topic is closed to new replies.

Advertisement