• FEATURED

View more

View more

View more

\$5

### Image of the Day Submit

IOTD | Top Screenshots

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

11 replies to this topic

### #1MarkS  Members

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.

#ifndef __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;

long GetVertex(long index) const;

long GetNormal(long index) const;

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;

char *GetOBJName() const;

private:
char			*obj_name;

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

#endif

#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());
}

{
vertices->push_back(index);
}

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

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

{
normals->push_back(index);
}

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

return(normals->at(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));
}

{
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;

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
break;
case EOF: // End of file reached. We be done.
done = true;
break;
c = fgetc(fs); // The next character determines what type of vertex we are loading
switch(c)
{
fscanf_s(fs,"%f %f %f\n",&x,&y,&z);
vert.SetX(x);
vert.SetY(y);
vert.SetZ(z);
vertices->push_back(vert);
break;
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;
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;

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.
}
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;

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;
}

}
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;

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;
}

}
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;

if((read_count != 1) || (vert_ind == 0))
{
SAFE_DELETE(temp_face);
SAFE_DELETE_A(obj_name);
cout << "Invalid vertex index." << endl;
return;
}

}
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.

### #2fastcall22  Moderators

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 0xBEEFCACE:
case 0xCCCCCCCC:
case 0xCDCDCDCD:
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

### #3MarkS  Members

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 0xBEEFCACE:
case 0xCCCCCCCC:
case 0xCDCDCDCD:
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.

### #4samoth  Members

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>

### #5LennyLen  GDNet+

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. ;)

### #6Kylotan  Moderators

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.

### #7MarkS  Members

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.

### #8frob  Moderators

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.

### #9l0calh05t  Members

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.

### #10Juliean  GDNet+

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 )

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.

### #11l0calh05t  Members

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 )

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".

### #12Juliean  GDNet+

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.