Jump to content

View more

Image of the Day

Adding some finishing touches...
Follow us for more
#screenshotsaturday #indiedev... by #MakeGoodGames https://t.co/Otbwywbm3a
IOTD | Top Screenshots

The latest, straight to your Inbox.

Subscribe to GameDev.net Direct to receive the latest updates and exclusive content.


Sign up now

My special blend of C and C++...

4: Adsense
  • You cannot reply to this topic
10 replies to this topic

#1 MarkS   Members   

3466
Like
4Likes
Like

Posted 08 January 2017 - 10:19 AM

I found this recently while looking through old code. It's an OBJ file loader that I wrote while learning about C++ containers. It's... special... It has just the right blend of C and C++ with a good dash of heap allocated containers and a hint of "using namespace std" for that special code smell aroma. Breath it in deeply.

Works perfectly, BTW.

OBJ Loader.h
#ifndef __OBJ_LOADER__
#define __OBJ_LOADER__

#include <vector>
#include <iostream>

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

using namespace std;

class vertex{
public:
	float GetX() const {return(x);}
	void SetX(float _x) {x = _x;}

	float GetY() const {return(y);}
	void SetY(float _y) {y = _y;}

	float GetZ() const {return(z);}
	void SetZ(float _z) {z = _z;}

private:
	float	x;
	float	y;
	float	z;
};

class uv_coords{
public:
	float GetU() const {return(u);}
	void SetU(float _u) {u = _u;}

	float GetV() const {return(v);}
	void SetV(float _v) {v = _v;}

private:
	float	u;
	float	v;
};

class face{
public:
	face();
	face(const face &right);
	~face();

	long GetNumVertices() const;
	long GetNumNormals() const;
	long GetNumUVs() const;

	void AddVertex(long index);
	long GetVertex(long index) const;

	void AddNormal(long index);
	long GetNormal(long index) const;

	void AddUV(long index);
	long GetUV(long index) const;

private:
	vector<long>	*vertices;
	vector<long>	*normals;
	vector<long>	*uv;
};

class OBJ{
public:
	OBJ();
	~OBJ();

	long GetNumVertices() const;
	long GetNumNormals() const;
	long GetNumUVs() const;
	long GetNumFaces() const;

	vertex *GetVertex(long index) const;
	vertex *GetNormal(long index) const;
	uv_coords *GetUV(long index) const;
	face *GetFace(long index) const;

	void LoadOBJ(char *file_name);

	char *GetOBJName() const;

private:
	char			*obj_name;

	vector<vertex>		*vertices;
	vector<vertex>		*normals;
	vector<uv_coords>	*uv;
	vector<face>		*faces;
};

#endif
OBJ Loader.cpp
#include "OBJ Loader.h"

#define SAFE_DELETE(p)		if(p){delete p; p = NULL;} // Delete pointer
#define SAFE_DELETE_A(p)	if(p){delete [] p; p = NULL;} // Delete array

face::face()
{
	vertices = new vector<long>;
	normals = new vector<long>;
	uv = new vector<long>;
}

face::face(const face &right)
{
	long	i;

	vertices = new vector<long>;
	normals = new vector<long>;
	uv = new vector<long>;

	for(i = 0;i < right.vertices->size();i++)
		vertices->push_back(right.vertices->at(i));
	for(i = 0;i < right.normals->size();i++)
		normals->push_back(right.normals->at(i));
	for(i = 0;i < right.uv->size();i++)
		uv->push_back(right.uv->at(i));
}

face::~face()
{
	SAFE_DELETE(vertices);

	SAFE_DELETE(normals);

	SAFE_DELETE(uv);
}

long face::GetNumVertices() const
{
	return(vertices->size());
}

long face::GetNumNormals() const
{
	return(normals->size());
}

long face::GetNumUVs() const
{
	return(uv->size());
}

void face::AddVertex(long index)
{
	vertices->push_back(index);
}

long face::GetVertex(long index) const
{
	if(index < 0 || index > vertices->size() - 1)
		return(NULL);

	return(vertices->at(index));
}

void face::AddNormal(long index)
{
	normals->push_back(index);
}

long face::GetNormal(long index) const
{
	if(index < 0 || index > normals->size() - 1)
		return(NULL);

	return(normals->at(index));
}

void face::AddUV(long index)
{
	uv->push_back(index);
}

long face::GetUV(long index) const
{
	if(index < 0 || index > uv->size() - 1)
		return(NULL);

	return(uv->at(index));
}

OBJ::OBJ()
{
	obj_name = NULL;

	vertices = new vector<vertex>;
	normals = new vector<vertex>;
	uv = new vector<uv_coords>;
	faces = new vector<face>;
}

OBJ::~OBJ()
{

	SAFE_DELETE_A(obj_name);

	SAFE_DELETE(vertices);

	SAFE_DELETE(normals);

	SAFE_DELETE(uv);

	SAFE_DELETE(faces);
}

long OBJ::GetNumVertices() const
{
	return(vertices->size());
}

long OBJ::GetNumNormals() const
{
	return(normals->size());
}

long OBJ::GetNumUVs() const
{
	return(uv->size());
}

long OBJ::GetNumFaces() const
{
	return(faces->size());
}

vertex *OBJ::GetVertex(long index) const
{
	if(index < 0 || index > vertices->size() - 1)
		return(NULL);

	return(&vertices->at(index));
}

vertex *OBJ::GetNormal(long index) const
{
	if(index < 0 || index > normals->size() - 1)
		return(NULL);

	return(&normals->at(index));
}

uv_coords *OBJ::GetUV(long index) const
{
	if(index < 0 || index > uv->size() - 1)
		return(NULL);

	return(&uv->at(index));
}

face *OBJ::GetFace(long index) const
{
	if(index < 0 || index > faces->size() - 1)
		return(NULL);

	return(&faces->at(index));
}

void OBJ::LoadOBJ(char *file_name)
{
	FILE		*fs;
	char		c;
	bool		done,has_normals,has_uvs;
	vertex		vert;
	uv_coords	uvs;
	face		*temp_face;
	float		x,y,z;
	float		u,v;
	int		vert_ind,norm_ind,uv_ind;
	int		read_count;

	if(fopen_s(&fs,file_name,"r") != 0)
		return;

	obj_name = new char[strlen(file_name) + 1];
	strcpy(obj_name,file_name);

	done = false;
	has_normals = false;
	has_uvs = false;

	while(!done)
	{
		c = fgetc(fs);
		switch(c)
		{
			case '#': // Comment. Fall through
			case 'u': // Fall through
			case 's': // Fall through
			case 'g': // Group. Not supported. Fall through
				while(fgetc(fs) != '\n'); // Skip to the next line
			break;
			case EOF: // End of file reached. We be done.
				done = true;
			break;
			case 'v': // Loading vertices is easy. Faces, not so much...
				c = fgetc(fs); // The next character determines what type of vertex we are loading
				switch(c)
				{
					case ' ': // Loading vertices
						fscanf_s(fs,"%f %f %f\n",&x,&y,&z);
						vert.SetX(x);
						vert.SetY(y);
						vert.SetZ(z);
						vertices->push_back(vert);
					break;
					case 'n': // Loading normals
						has_normals = true;
						fscanf_s(fs,"%f %f %f\n",&x,&y,&z);
						vert.SetX(x);
						vert.SetY(y);
						vert.SetZ(z);
						normals->push_back(vert);
					break;
					case 't': // Loading UVs
						has_uvs = true;
						fscanf_s(fs,"%f %f\n",&u,&v);
						uvs.SetU(u);
						uvs.SetV(v);
						uv->push_back(uvs);
					break;
					default: // Uh oh... What are we trying to read here? Someone screwed up their OBJ exporter...
						cout << "Invalid vertex type: " << "v" << c << " Should be of type \"v \", \"vn\" or \"vt\"." << endl;
						SAFE_DELETE_A(obj_name);
						return;
				}
			break;
			case 'f':
				if(has_normals && has_uvs)
				{
					temp_face = new face;
					while((c = fgetc(fs)) != '\n')
					{
						vert_ind = 0;
						norm_ind = 0;
						uv_ind = 0;

						read_count = fscanf_s(fs,"%d/%d/%d",&vert_ind,&uv_ind,&norm_ind);
						if((read_count != 3) || (vert_ind == 0) || (uv_ind == 0) || (norm_ind == 0)) // Valid indices start at 1 in a .OBJ file. Don't know why... O.o
						{
							SAFE_DELETE(temp_face);
							SAFE_DELETE_A(obj_name);
							cout << "Invalid vertex index." << endl;
							return;
						}

						temp_face->AddVertex(vert_ind - 1); // We want the indices to start at 0, so subtract 1.
						temp_face->AddNormal(norm_ind - 1);
						temp_face->AddUV(uv_ind - 1);
					}
					faces->push_back(*temp_face);

					SAFE_DELETE(temp_face);
				}else if(has_normals && !has_uvs){
					temp_face = new face;
					while((c = fgetc(fs)) != '\n')
					{
						vert_ind = 0;
						norm_ind = 0;

						read_count = fscanf_s(fs,"%d//%d",&vert_ind,&norm_ind);
						if((read_count != 2) || (vert_ind == 0) || (norm_ind == 0))
						{
							SAFE_DELETE(temp_face);
							SAFE_DELETE_A(obj_name);
							cout << "Invalid vertex index." << endl;
							return;
						}

						temp_face->AddVertex(vert_ind - 1);
						temp_face->AddNormal(norm_ind - 1);
						temp_face->AddUV(0);
					}
					faces->push_back(*temp_face);

					SAFE_DELETE(temp_face);
				}else if(!has_normals && has_uvs){
					temp_face = new face;
					while((c = fgetc(fs)) != '\n')
					{
						vert_ind = 0;
						uv_ind = 0;

						read_count = fscanf_s(fs,"%d/%d/",&vert_ind,&uv_ind);
						if((read_count != 2) || (vert_ind == 0) || (uv_ind == 0))
						{
							SAFE_DELETE(temp_face);
							SAFE_DELETE_A(obj_name);
							cout << "Invalid vertex index." << endl;
							return;
						}

						temp_face->AddVertex(vert_ind - 1);
						temp_face->AddNormal(0);
						temp_face->AddUV(uv_ind - 1);
					}
					faces->push_back(*temp_face);

					SAFE_DELETE(temp_face);
				}else if(!has_normals && !has_uvs){
					temp_face = new face;
					while((c = fgetc(fs)) != '\n')
					{
						vert_ind = 0;

						read_count = fscanf_s(fs,"%d//",&vert_ind);
						if((read_count != 1) || (vert_ind == 0))
						{
							SAFE_DELETE(temp_face);
							SAFE_DELETE_A(obj_name);
							cout << "Invalid vertex index." << endl;
							return;
						}

						temp_face->AddVertex(vert_ind - 1);
						temp_face->AddNormal(0);
						temp_face->AddUV(0);
					}
					faces->push_back(*temp_face);

					SAFE_DELETE(temp_face);
				}
			break;
		}
	}

	fclose(fs);
}

char *OBJ::GetOBJName() const
{
	return(obj_name);
}

Edited by MarkS, 08 January 2017 - 10:21 AM.


#2 fastcall22   Moderators   

10779
Like
5Likes
Like

Posted 08 January 2017 - 10:45 AM

*
POPULAR

Ah yes, the C–with–classes phase. Your SAFE_DELETE reminds me of something I used to do when I was going through this phase. I made an even safer [tt]SAFE_DELETE[tt] that went something like this:
 
void safe_delete(void* ptr)
{
	switch ( ptr )
	{
	case 0:
	case 0xABABABAB:
	case 0xABADCAFE:
	case 0xBAADF00D:
	case 0xBADCAB1E:
	case 0xBEEFCACE:
	case 0xCCCCCCCC:
	case 0xCDCDCDCD:
	case 0xDEADDEAD:
	case 0xFDFDFDFD:
	case 0xFEEEFEEE:
		return;
	}

	delete ptr;
}

// And then something like
#define SAFE_DELETE(p) safe_delete(p); p = NULL;
It didn’t occur to me when writing this that the deletion problem must be elsewhere in my code.

Edited by fastcall22, 08 January 2017 - 01:44 PM.
Forgot the accompanying macro (yep)

zlib: eJzVVLsSAiEQ6/1qCwoK i7PxA/2S2zMOZljYB1TO ZG7OhUtiduH9egZQCJH9 KcJyo4Wq9t0/RXkKmjx+ cgU4FIMWHhKCU+o/Nx2R LEPgQWLtnfcErbiEl0u4 0UrMghhZewgYcptoEF42 YMj+Z1kg+bVvqxhyo17h nUf+h4b2W4bR4XO01TJ7 qFNzA7jjbxyL71Avh6Tv odnFk4hnxxAf4w6496Kd OgH7/RxC

#3 MarkS   Members   

3466
Like
4Likes
Like

Posted 08 January 2017 - 10:50 AM

Ah yes, the C–with–classes phase. Your SAFE_DELETE reminds me of something I used to do when I was going through this phase. I made an even safer [tt]SAFE_DELETE[tt] that went something like this:
 

void safe_delete(void* ptr)
{
	switch ( ptr )
	{
	case 0:
	case 0xABABABAB:
	case 0xABADCAFE:
	case 0xBAADF00D:
	case 0xBADCAB1E:
	case 0xBEEFCACE:
	case 0xCCCCCCCC:
	case 0xCDCDCDCD:
	case 0xDEADDEAD:
	case 0xFDFDFDFD:
	case 0xFEEEFEEE:
		return;
	}

	delete ptr;
}
It didn’t occur to me when writing this that the deletion problem must be elsewhere in my code.


You forgot 0xDEADBEEF.

#4 samoth   Members   

9739
Like
4Likes
Like

Posted 08 January 2017 - 01:21 PM

I actually wrote a version of SAFE_DELETE myself in one project, a long, long time ago. That was to fix a crash, and successfully fix the crash it did!

It didn't occur to me at that time that when you need to set pointers to nullptr after deleting, then your program logic is totally fucked. It didn't occur to me that double deletes crashing hard was actually a good thing, either.

EDIT: <loosely related c++ rant>
By the way, the C++ committee has put a lot of effort to make writing code that crashes in a meaningful way (with constants as the ones in the previous post) as hard as possible with C++14.

I noticed first when writing a cross-platform lib which uses among other things... well, "operating system handles" of some kind, which are void pointers under Windows. There are special values that designate special intentions or conditions. One such special value is -1 cast to the respective type. Now, -1 is a perfectly good compiletime constant, is it not. Just like 0xdeadbeef would be. Thus, the obvious thing is something like:

constexpr handle_t invalid_handle_value = reinterpret_cast<handle_t>(-1);

Guess what the compiler tells you (regardless of what type of cast you attempt):

constexpr variable 'invalid_handle_value' must be initialized by a constant expression
   note: reinterpret_cast is not allowed in a constant expression

Great. Thank you. First I thought it was a compiler bug, but this error indeed follows exactly the explicit wording of the standard.

 

So, we're back to #define invalid_handle_value ((void*)-1). Or we can use a normal const variable, for which the compiler will needlessly allocate an address and room in the executable's rdata section. For a compiletime constant. Very intelligent, thank you again. Much better now.

 

Guess what the compiler tells you when you try something like constexpr handle_t invalid_handle_value =  &((char*)nullptr)[-1]; to work around this :)

Apart from obviously invoking undefined behavior, this is not a constant expression either...
</loosely related c++ rant>



#5 LennyLen   GDNet+   

5692
Like
2Likes
Like

Posted 10 January 2017 - 03:56 AM

Ah yes, the C–with–classes phase

 

It's a phase?  Damn, I've been going through that phase for about 20 years then. ;) 



#6 Kylotan   Moderators   

9280
Like
2Likes
Like

Posted 10 January 2017 - 05:06 AM

Looks like what a 90s Java programmer would make if told they had to get something working in C++ in a day. Everything 'new'ed, everything gettered-and-settered.



#7 MarkS   Members   

3466
Like
1Likes
Like

Posted 10 January 2017 - 01:40 PM

Looks like what a 90s Java programmer would make if told they had to get something working in C++ in a day. Everything 'new'ed, everything gettered-and-settered.


I was just getting my feet wet after years of resistance to C++ and OOP. I'm still resistant to OOP, but slowly coming around. I didn't understand that the purpose of the containers was to remove the need for new/delete and it never occurred to me that if I'm exposing the member to the user via get/set, then it probably needs to be public.

#8 frob   Moderators   

44436
Like
2Likes
Like

Posted 10 January 2017 - 03:58 PM

it never occurred to me that if I'm exposing the member to the user via get/set, then it probably needs to be public.

 

It actually depends on the nature of the class.

 

For classes whose single responsibility is to contain data, you are right that they should generally just have public data fields.  Mutators and accessors don't make sense for these.

 

For classes whose single responsibility is actions, the functions should typically be actions.  They should be verbs, DO something.  In the rare case where the inner guts of the object needs to be exposed -- and it should be rare in well-written code -- then proper accessor and mutator methods are the right choice.  They generally indicate that you probably shouldn't be mucking about with the object internals. 


Check out my book, Game Development with Unity, aimed at beginners who want to build fun games fast.

Also check out my personal website at bryanwagstaff.com, where I occasionally write about assorted stuff.


#9 l0calh05t   Members   

1785
Like
1Likes
Like

Posted 11 January 2017 - 02:40 AM

I actually wrote a version of SAFE_DELETE myself in one project, a long, long time ago. That was to fix a crash, and successfully fix the crash it did!

It didn't occur to me at that time that when you need to set pointers to nullptr after deleting, then your program logic is totally fucked. It didn't occur to me that double deletes crashing hard was actually a good thing, either.

EDIT: <loosely related c++ rant>
By the way, the C++ committee has put a lot of effort to make writing code that crashes in a meaningful way (with constants as the ones in the previous post) as hard as possible with C++14.

I noticed first when writing a cross-platform lib which uses among other things... well, "operating system handles" of some kind, which are void pointers under Windows. There are special values that designate special intentions or conditions. One such special value is -1 cast to the respective type. Now, -1 is a perfectly good compiletime constant, is it not. Just like 0xdeadbeef would be. Thus, the obvious thing is something like:

constexpr handle_t invalid_handle_value = reinterpret_cast<handle_t>(-1);

Guess what the compiler tells you (regardless of what type of cast you attempt):

constexpr variable 'invalid_handle_value' must be initialized by a constant expression
   note: reinterpret_cast is not allowed in a constant expression

Great. Thank you. First I thought it was a compiler bug, but this error indeed follows exactly the explicit wording of the standard.

 

So, we're back to #define invalid_handle_value ((void*)-1). Or we can use a normal const variable, for which the compiler will needlessly allocate an address and room in the executable's rdata section. For a compiletime constant. Very intelligent, thank you again. Much better now.

 

Guess what the compiler tells you when you try something like constexpr handle_t invalid_handle_value =  &((char*)nullptr)[-1]; to work around this :)

Apart from obviously invoking undefined behavior, this is not a constant expression either...
</loosely related c++ rant>

 

Or, since you don't actually need the constexpr, you could just write a small inline function:

inline const handle_t invalid_handle() { return reinterpret_cast<handle_t>(-1); }

It will be inlined by just about every compiler out there on the lowest (non-disabled) optimization settings, so no additional global will be allocated. Many of the constants in std::numeric_limits are actually functions.



#10 Juliean   Members   

7004
Like
1Likes
Like

Posted 14 January 2017 - 08:17 AM

EDIT: Ok, I just read that constexpr wasn't possible in that case. I'll leave my answer on as it still a valid point towards l0calh05t reply

 

Or, since you don't actually need the constexpr, you could just write a small inline function: inline const handle_t invalid_handle() { return reinterpret_cast(-1); } It will be inlined by just about every compiler out there on the lowest (non-disabled) optimization settings, so no additional global will be allocated. Many of the constants in std::numeric_limits are actually functions.

 

1) A constexpr variable will be fully evaluated at compile-time, so there should be no real difference between a constexpr function and a variable.

 

2) Therefore, if you actually replace a constexpr-variable, do it with a constexpr function. While the compiler can inline your function, constexpr ensures that it will. Actually a constexpr-function will be resolved to a value at compile-time, so this can be enormously faster than even an inlined function (since no code has to be executed for the function at all).

 

;TL;DR: constexpr > compiler inlining (if you can actually use it :D )

 

Great. Thank you. First I thought it was a compiler bug, but this error indeed follows exactly the explicit wording of the standard.

 

Yeah, unfortunately if the type of a constexpr-statement is not a primitive type, you are out of luck. You can define a constexpr-ctor, but I most (platform)-libraries actually won't do that.

Out of interest, whats the exact type of handle_t in your example?


Edited by Juliean, 14 January 2017 - 08:22 AM.


#11 l0calh05t   Members   

1785
Like
1Likes
Like

Posted 17 January 2017 - 12:41 AM

EDIT: Ok, I just read that constexpr wasn't possible in that case. I'll leave my answer on as it still a valid point towards l0calh05t reply

 

Or, since you don't actually need the constexpr, you could just write a small inline function: inline const handle_t invalid_handle() { return reinterpret_cast(-1); } It will be inlined by just about every compiler out there on the lowest (non-disabled) optimization settings, so no additional global will be allocated. Many of the constants in std::numeric_limits are actually functions.

 

1) A constexpr variable will be fully evaluated at compile-time, so there should be no real difference between a constexpr function and a variable.

 

2) Therefore, if you actually replace a constexpr-variable, do it with a constexpr function. While the compiler can inline your function, constexpr ensures that it will. Actually a constexpr-function will be resolved to a value at compile-time, so this can be enormously faster than even an inlined function (since no code has to be executed for the function at all).

 

;TL;DR: constexpr > compiler inlining (if you can actually use it :D )

 

Which you can't in this case, ergo const function provided inline. Also, constepxr doesn't guarantee inlining either BTW. Unless evaluated into a constexpr "variable".



#12 Juliean   Members   

7004
Like
1Likes
Like

Posted 17 January 2017 - 05:57 AM

Also, constepxr doesn't guarantee inlining either BTW. Unless evaluated into a constexpr "variable".

 

True, yet its easier to control/check, and a constexpr-function with zero-parameters that is once checked to produce a constexpr-output will AFAIK always be "inlined". In this case its not important though since it cannot be applied - I'm still curious whats the actual type of handle_t is, and if there is not a way to enforce a variable with constexpr nevertheless.