Sign in to follow this  

[C++] What's wrong with these classes?

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

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello everyone! Can someone take a look at my code and tell me what's wrong with it? It doesn't compile on VC++ or GCC... [sad] Error.h
#pragma once

#include "Includes.h"

class Error
{
	static char lastError[512];

public:
	static char *GetLastError();
	static void SetError(char* error);
};



Error.cpp
#include "Includes.h"
#include "Error.h"

char *Error::GetLastError()
{
	return Error::lastError;
}

void Error::SetError(char *error)
{
	strcpy(Error::lastError, error);
}



Map.h
#pragma once

#include "Includes.h"

// TODO: Move to Structures.h maybe?
struct TileFile
{
	/* surf:		SDL surface
	   s:			Tile size
	   w/h:			How much tiles it has
     */
	SDL_Surface *layers[4];	// Underground, Ground, Building Floor 2, and Building Floor 3
	int s, w, h;
};

class Map
{
public:
	Map(void);
	~Map(void);

	// TODO: Make as a global var in TileFile declaration?
	static TileFile Tilesets[2];

	static bool LoadTiles();
};



Map.cpp
#include "Includes.h"
#include "Map.h"

Map::Map(void)
{
}

Map::~Map(void)
{
}

bool Map::LoadTiles()
{
	// Main tiles
	Map::Tilesets[0].layers[1] = IMG_Load("./Graphics/Tiles.png");
	Map::Tilesets[0].s = 24;
	Map::Tilesets[0].w =  3;
	Map::Tilesets[0].h =  2;

	if (!Map::Tilesets[0].layers[1])
	{
		return false;
	}

	if (!Map::Tilesets[0].layers[1])
	{
		Error::SetError(IMG_GetError());
		return false;
	}

	return true;
}



Help! [sad] Thanks! [Edited by - Kixdemp on September 15, 2006 6:29:01 PM]

Share this post


Link to post
Share on other sites
The only thing I see is that map.cpp isn't actually including error.h, but is trying to refer to the error class.

Also, error.h exists already to hold DOS error codes, so who knows which one it will find first when you do add the include. I'd rename it before it causes trouble. Generic filenames are evil.

Generic class names are evil too... Try embedding your renderer inside Max or Maya when you've called your Camera "Camera". I'd bet class Error is asking for trouble. I know class Map is asking for trouble. Doesn't STL have a map class in it?

Share this post


Link to post
Share on other sites
It looks like you're never initializing your static variables. For example, at the top of Error.cpp, you need:

char Error::lastError[512];


Quote:
Generic class names are evil too... Try embedding your renderer inside Max or Maya when you've called your Camera "Camera". I'd bet class Error is asking for trouble.

I used to agree, but hey...that's what namespaces are for. Put everything under your own namespace, and you can be as lazy and uncreative as you want!

Share this post


Link to post
Share on other sites
Quote:
Also, error.h exists already to hold DOS error codes, so who knows which one it will find first when you do add the include.


I do. It's perfectly well-defined.

If I include it using '#include "error.h"', it will find the local file.
If I use '#include <error.h>', it will find the library file.

Share this post


Link to post
Share on other sites
Oh my god.

Please promise me you will never try to handle errors, or text manipulation, or object creation, anything remotely like that in C++ ever again, without *exceptional* circumstances (pun intended, for those of you who know what's coming up).

0) Use constructors.
0a) Give TileFile a constructor. Yes, in C++, structs can have ctors, dtors, and actually anything classes can have. The only difference is the default for public vs. private access and inheritance.
0a) Use initializer lists.
0b) Don't write out destructors if they're not going to do anything (i.e. the body is empty and you don't need to specify they are virtual).
0c) Report errors in constructors by throwing an exception. Definitely *don't* set up a "get last error" interface; these things are unbelievably error-prone (ironically enough). Ever get an error dialog telling you no error occurred, or the command completed successfully? That's because the Windows API does this nonsense (because it had to interface with a LOT of pre-existing C code). You don't have these limitations, so don't work around them.
1) Use std::string to represent text (except where you have a string literal and don't need to specify a type, or you have text "symbols" that will never need to change - there are other reasons, but they're considerably more obscure, and you'll recognize them when you run into them).
2) Don't write (void) as a function prototype, unless you have to interface with C code (and even then...). Write () instead.
3) If you're going to let the "Map" just be a holder of static instances of stuff, then don't use a class; use a namespace.
4) Don't create a global "Includes.h" file (unless it's a standardized PCH, like stdafx.h for Windows programs). You're just asking for linker problems that way.
4a) Don't let #pragma once substitute for include guards; use the include guards too, because pragmas are not portable.

Map.h - don't need any of the others after these changes:

#pragma once
#include "Includes.h" // FIXME
#include <string>

// TODO: Move to Structures.h maybe?
struct TileFile
{
SDL_Surface *layers[4];
// Underground, Ground, Building Floor 2, and Building Floor 3
// Instead of making single-letter names and then commenting them at
// length, why not just give them names that you can understand when
// you see them?
int size, width, height;

TileFile() : size(0), width(0), h(0) {}

TileFile(int size, int width, int height, const std::string& filename) :
size(size), width(width), height(height) {
layers[1] = IMG_Load(filename.c_str());
// you might consider nulling out the other pointers? :s
if (!layers[1])
{
throw std::exception(IMG_GetError());
// restrain SDL by stopping the madness here.
// TODO: create our own exception subclass.
}
}

~TileFile() {
IMG_Unload(layers[1]);
// probably you'll want to do that for all of them.
// In general, you want to allocate your resources in the ctor
// and deallocate them in the dtor.
}
};

namespace Map
{
// When there gets to be evidence of this needing to be a class, we
// can talk some more.

TileFile Tilesets[2];

void LoadTiles() {
// We don't need 'Map::' because we're already in that namespace
Tilesets[0] = TileFile(24, 3, 2, "./Graphics/Tiles.png");
// if an exception happens, we just let it fall through.
}
};

Share this post


Link to post
Share on other sites
ARGH. $^*$ (@&$ (@&$ F*&) *^*(^ *$^!!!!!!!
Your code seemed to be the fix, but it still screws around... [tears]
Sorry about not posting the error messages, I thought I did it when I edited the message to put the Notify by Email checkbox... I probably forgot to press the Submit button... [embarrass] But no, these are the errors:


1>------ Build started: Project: War Game, Configuration: Release Win32 ------
1>Compiling...
1>MapHandling.cpp
1>ErrorHandling.cpp
1>Vehicle.cpp
1>Player.cpp
1>Main.cpp
1>HelperFuncs.cpp
1>Game.cpp
1>Linking...
1>HelperFuncs.obj : error LNK2005: "void __cdecl MapHandling::LoadTiles(void)" (?LoadTiles@MapHandling@@YAXXZ) already defined in Game.obj
1>HelperFuncs.obj : error LNK2005: "class _WGError WGError" (?WGError@@3V_WGError@@A) already defined in Game.obj
1>HelperFuncs.obj : error LNK2005: "struct TileFile * MapHandling::Tilesets" (?Tilesets@MapHandling@@3PAUTileFile@@A) already defined in Game.obj
1>Main.obj : error LNK2005: "void __cdecl MapHandling::LoadTiles(void)" (?LoadTiles@MapHandling@@YAXXZ) already defined in Game.obj
1>Main.obj : error LNK2005: "class _WGError WGError" (?WGError@@3V_WGError@@A) already defined in Game.obj
1>Main.obj : error LNK2005: "struct TileFile * MapHandling::Tilesets" (?Tilesets@MapHandling@@3PAUTileFile@@A) already defined in Game.obj
1>Player.obj : error LNK2005: "void __cdecl MapHandling::LoadTiles(void)" (?LoadTiles@MapHandling@@YAXXZ) already defined in Game.obj
1>Player.obj : error LNK2005: "class _WGError WGError" (?WGError@@3V_WGError@@A) already defined in Game.obj
1>Player.obj : error LNK2005: "struct TileFile * MapHandling::Tilesets" (?Tilesets@MapHandling@@3PAUTileFile@@A) already defined in Game.obj
1>Vehicle.obj : error LNK2005: "void __cdecl MapHandling::LoadTiles(void)" (?LoadTiles@MapHandling@@YAXXZ) already defined in Game.obj
1>Vehicle.obj : error LNK2005: "class _WGError WGError" (?WGError@@3V_WGError@@A) already defined in Game.obj
1>Vehicle.obj : error LNK2005: "struct TileFile * MapHandling::Tilesets" (?Tilesets@MapHandling@@3PAUTileFile@@A) already defined in Game.obj
1>ErrorHandling.obj : error LNK2005: "void __cdecl MapHandling::LoadTiles(void)" (?LoadTiles@MapHandling@@YAXXZ) already defined in Game.obj
1>ErrorHandling.obj : error LNK2005: "class _WGError WGError" (?WGError@@3V_WGError@@A) already defined in Game.obj
1>ErrorHandling.obj : error LNK2005: "struct TileFile * MapHandling::Tilesets" (?Tilesets@MapHandling@@3PAUTileFile@@A) already defined in Game.obj
1>MapHandling.obj : error LNK2005: "void __cdecl MapHandling::LoadTiles(void)" (?LoadTiles@MapHandling@@YAXXZ) already defined in Game.obj
1>MapHandling.obj : error LNK2005: "class _WGError WGError" (?WGError@@3V_WGError@@A) already defined in Game.obj
1>MapHandling.obj : error LNK2005: "struct TileFile * MapHandling::Tilesets" (?Tilesets@MapHandling@@3PAUTileFile@@A) already defined in Game.obj
1>..\Release/War Game.exe : fatal error LNK1169: one or more multiply defined symbols found
1>Build log was saved at "file://c:\Incoming\War Game\Visual Studio 2005\Release\BuildLog.htm"
1>War Game - 19 error(s), 0 warning(s)
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========




As you can see, they're all related to those 2 files... Any ideas what it could be? Thanks!

(PS: I learned a lot from you guys today... Thanks! [grin])

[edit] Also, after much plain-C coding, I had completely forgotten about std::string... [/edit]

[edit2] Now, I switched to exceptions (forgotten that too... [embarrass]), did a lot of change in my code, renamed the files, it gets to the Linking part, but STILL I get those errors... [/edit2]

[edit3] Updated the errors. [/edit3]

[Edited by - Kixdemp on September 15, 2006 6:55:19 PM]

Share this post


Link to post
Share on other sites

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

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this