The C++ WTF why doesn't it work?!?! thread

[Ronin Storm]

Somedays, I hate programming.

Just working through the chapter on references and as part of an exercise I've created myself a tiny test class that I can instantiate. Here's the parts:

TestObject.h

class TestObject
{
public:
	TestObject();
	~TestObject();
	void SetI(int i);
	int GetI() const;
private:
	int i;
}

TestObject.cpp

#include "TestObject.h"

TestObject::TestObject()
{
	this->i = 1;
}

TestObject::~TestObject()
{
}

void TestObject::SetI(int i)
{
	this->i = i;
}

int TestObject::GetI() const
{
	return this->i;
}

Visual Studio tells me:

Error 1 error C2533: 'TestObject::{ctor}' : constructors not allowed a return type c:\documents and settings\paul marshall\my documents\_development\cpp learning\chapter8\c8exercises\testobject.cpp 4

MSDN doesn't give any more useful information. Anyone see where I've specified a return type for my constructor? Hmm? NOWHERE! Stupid dumbass code.

Help?

 

[Ronin Storm]

Ah, just figured it. Stupid lying compiler.

The header file needed a semi-colon at the end of the class definition, which was made obvious when I the compiler correctly reported that when I included the class header elsewhere in my code.

So, corrected:

TestObject.h

class TestObject
{
public:
	TestObject();
	~TestObject();
	void SetI(int i);
	int GetI() const;
private:
	int i;
};

 

[thatbloke]

I was just gonna post to say that...

 

[Ronin Storm]

Your psychic intervention saved me.

 

[Ronin Storm]

Different problem now. I want to create a multi-dimensional array on the heap.

So, I try this:

#include <iostream>

int main()
{
	const int max = 3;
	int * board = new int[3][3];
	int i = 0, j = 0;

	for (i = 0; i < max; i++)
	{
		for (j = 0; j < max; j++)
		{
			board[i][j] = 0;
		}
	}

	return 0;
}

Except that I get the following compiler errors:

Error 1 error C2440: 'initializing' : cannot convert from 'int (*)[3]' to 'int *' c:\documents and settings\paul marshall\my documents\_development\cpp learning\chapter13\chapter13\main.cpp 6

Error 2 error C2109: subscript requires array or pointer type c:\documents and settings\paul marshall\my documents\_development\cpp learning\chapter13\chapter13\main.cpp 13

I imagine that the second is related to the first so if I get the first solved then the second will fall in line.

If I declare a single dimension array we're all good. It's just the multi-dimension array that's giving me trouble. What gives?

 

[Taffy]

[mod]Stickied. Can go hand-in-hand with the learning thread.[/mod]

 

[elDiablo]

Well Ronin, it's because you are defining a multi-dimensional array with your int[3][3], but you are only declaring a single dimensional array with your int * board. You merely need to put an * for each dimension of your array! Therefore, change line 6

int * board = new int[3][3];

to

int ** board = new int[3][3];

and you should be laughing!

Edit - So, you merely need to put an * for each [x] you use.

int *** board = new int[3][3][3]

etc. And for your larger projects, don't forget about your memory leaks! I can talk to you about this more if you wish, but for now (so you can learn), I would like you to rewrite and post the code you put (whenever you wish) to deal with your memory leak

 

[thatbloke]

Memory leaks = bad!

hehehe

I have never caused a memory leak in my life!*

* May not be 100% true.

 

[Ronin Storm]

Ah, right, that makes a certain sense (and isn't in my book to date). Admittedly I was making this harder for myself than it needed to be; the exercise didn't ask for the array to be on the heap.

When cleaning this array up, given that it's multidimensional, do I now need to:

delete [][] pArray;

... or just:

delete [] pArray;

?

(The memory leak is there because I was compiling as I was writing and that got me. I admit failing to clean up my memory or pointers, though in the scope of this program the memory leak was going to last all of a few milliseconds before the entire process' memory was returned.)

 

[elDiablo]

Ok, I was talking through my behind and thinking of Java. My mistake!

To allocate multi-dimension arrays, you do need to specific the declaration with a number of *'s equal to the number of dimensions you use (so int ** board), but it's not as simple as saying "new int[3][3]", due to c++ being so "nice".

As a semi aside, unfortunately, there is no method defined as "delete [][]" (as other there would have to be delete [][][], and delete [][][][], etc.), which means you have to iterate through all sub-arrays, delete[]-ing them first, and then finally delete[]-ing the array of arrays. Fun times! See the examples below.

There are various ways you can initialise your multidimensional array, but I'll show you 2 methods, which might be a bit complex, I dunno!

Firstly, you can use malloc!

int ** board = (int**)malloc(3 * sizeof(int*));
for( int i = 0; i < 3; i++ )
{
    board[i] = new int[3];
}

...

for( int i = 0; i < 3; i++ )
{
    delete [] board[i];
}

delete [] board;

As you can see, you have to define each array within a loop after first malloc-ing your multidimensional array. You can extend this to even more dimensionals with more nested for loops. Sucks, but that's the way I learnt it...

The second method, which I'm not so familiar with, and don't really like, is to use a typedef. For examples:

typedef int (*IntArray)[3][3];
IntArray board = (IntArray) new int[3][3];

This looks a lot nicer, in principle, but I really don't like it as it hides a lot of what is going on behind the scenes with pointers and what nots. If you prefer this, then that's cool However, you need to dereference your board variable every time you want to use it. For example

(*board)[2][1] = 0;

As for deleting this, I'm not really sure. I've just compile this example (literally) into my current project to see if it works, and any combination of the delete[] with nested for loops I use seems to go BOOM. I can get it to delete[] the board object, but again, it's only delete[]-ing the first array of arrays, but subarrays, leading to MEMORY LEAKS OF DOOM.

So, I hope this is more informative to you! Enjoy!

Edit: More ramblings

You can also do

int (*board)[3] = new int[3][3];

if don't want dynamic sizes. Or even (from the C++ FAQ Lite), you can use a clever single dimension array (if you REALLY want to f**k with people trying to read your code):

int width = 3;
int height = 3;
int board = new int[width * height];
for( int i = 0; i < width; i++ )
{
    for( int j = 0; j < height; j++ )
    {
        board[width*i + j] = 0;
    }
}

God you'd be a bastard to use that all the time...

 

[Ronin Storm]

Yeah, my little bit of reading uncovered the array[x * y] method which, as you say, looks evil but very in keeping with the way the array memory is actually allocated. After all:

int board[3][3] = { 1, 2, 3, 4, 5, 6, 7, 8, 9};

That works just fine as initialisation for a board on the stack as the memory for the rows and columns is contiguous.

Thus,

int board[3 * 3] = { 1, 2, 3, 4, 5, 6, 7, 8, 9};

... should be equivalent, no?

The access to it, of course, is pretty unfriendly. Bah, back to the stack I go. Easier that way for this nature of problem. If I want a cunning collection I'm sure I can dig one out of the standard library or something...

 

[elDiablo]

Indeed on everything you have said! Both initialisations of the int arrays are fine.

 

[Ronin Storm]

(Wraith, all this makes sense if you read the book and follow along with the workshop! )

So...

Following on from my earlier touch on arrays, here's some character arrays. The mission is to join together three arrays that contain my first, middle and last names to give my full name in a fourth array. I can do that but there I was being good and cleaning up after myself and that bit doesn't work.

#include <iostream>
#include <string.h>

int main()
{
	using std::cout;
	using std::endl;

	char * firstName = "Paul";
	char * middleInitial = "D";
	char * lastName = "Marshall";

	// how long does the full name char array need to be?
	int firstLength = strlen(firstName);
	int initialLength = strlen(middleInitial);
	int lastLength = strlen(lastName);
	int fullLength = firstLength + initialLength + lastLength + 2;  // +2 to allow for spaces

	// create the array with the correct size and initialise to null characters
	char * fullName = new char[fullLength];
	for (unsigned short int i = 0; i < fullLength; i++)
	{
		fullName[i] = '\0';
	}

	// concat the strings into the new char array
	strcat(fullName, firstName);
	strcat(fullName, " ");
	strcat(fullName, middleInitial);
	strcat(fullName, " ");
	strcat(fullName, lastName);

	// spit out the full name
	cout << "Full name is ";
	for (int j = 0; j < fullLength; j++)
	{
		cout << fullName[j];
	}
	cout << "." << endl;

	// clean up
	delete [] firstName;
	delete [] middleInitial;
	delete [] lastName;
	delete [] fullName;

	firstName = 0;
	middleInitial = 0;
	lastName = 0;
	fullName = 0;

	return 0;
}

So, yeah, I get all the way to "delete [] firstName;" and the execution freezes. "delete firstName;" doesn't help either but as I'm pretty sure I've got an array on the end of that pointer I reckon I need "delete []".

Why oh why is my code freezing at that point? No exception, no error, just frozen.

 

[thatbloke]

elDiablo will correct me if I am wrong here, but as far as I can tell, because of the way you have allocated your char arrays you do not actually need to free them up, and this is why your program is hanging.

Rather than using new or malloc to allocate the chars you have created them as local variables. It may help a little more to think of you having initialised the char array firstName like this:

char firstName[] = "Paul";

This is (i believe) precisely the same as the first one. When you allocate a char array like this it is a locally allocated variable, meaning the scope of it is only within that function. When you exit that function (in this case the main function) all the locally allocated variables are destroyed. This includes your char array because you have not manually allocated the memory yourself with either new or malloc.

Also, that loop you have to print out the full name? horrible. Simply horrible. You don't need it.

The line above it can be extended so that instead of the loop after the "Full name is" printout line you have the following:

cout << "Full name is " << fullName;

or if you really wanted you could do this:

cout << "Full name is " << firstName << " " << middleInitial << " " << lastName << "." << endl;

Take this with a grain of salt as i may be slightly under the influence of alcohol right now

 

[Ronin Storm]

Rather than using new or malloc to allocate the chars you have created them as local variables.

Ah weird, so I've got a constant pointer to an address on the stack not the heap, right? If that's the case, I can certainly see why I don't need to delete the memory allocated because it'll obviously go out of scope just as anything does on the stack when the stack pointer falls past it.

Also, that loop you have to print out the full name? horrible. Simply horrible. You don't need it.

Ah, gotcha. See, I first tried:

cout << "Full name is " << *fullName;

But then all I got was:

Full name is P

Hence the loop. This tells me I still haven't quite got arrays and pointers to arrays sorted out yet. Needs more work.

Understand on the second option but the exercise was quite specific about using arrays so I'd be missing the point to just drop the values out.

Thanks!

 

[thatbloke]

Ah, gotcha. See, I first tried:

cout << "Full name is " << *fullName;

But then all I got was:

Full name is P

Hence the loop. This tells me I still haven't quite got arrays and pointers to arrays sorted out yet. Needs more work.

while fullName itself is actually a pointer, it is a pointer to the first byte of memory that has been allocated for it. So de-referencing fullName by putting the * in front of it means that you will get the first part of memory that it points to, in this case the P which is your first initial. The idea of cout (and its predecessor printf) is that it takes (null-terminated) pointers to the areas of memory you want to print out, otherwise you will just get the initial chunk.

if that makes sense. it's early.

 

[elDiablo]

I need not correct you at all bloke, you hit all the proverbial nails on their heads.

It's a bit weird thinking of arrays as pointers and vice versa in C++ to start with, but when you get used to the whole & and * operators, you'll be fine!

As a small bit of info to help you learn, you also don't need the first loop for filling your fullName array with null terminators. This is because of the way strcat works:

char * strcat ( char * destination, const char * source );

Concatenate strings

Appends a copy of the source string to the destination string. The terminating null character in destination is overwritten by the first character of source, and a new null-character is appended at the end of the new string formed by the concatenation of both in destination.

(from the very useful C++ reference site). As the trailing null terminator of destination, you need only put a null terminator at the start of your array, namely:

fullName[0] = '\0';

Obviously not a huge gain in this program, but for larger projects that do string manipulation a lot, it's worth using. As an aside other str operations may need the null terminator at the END of the array, in which case you just need to set that single terminator

The other method is to use memset, which fill a block of memory with a specified value. I'm not sure how it does it, but it's usually fast than a for loop iterating over every value as it's optimised as such.

Finally, while using strcat is again fine for this code, it works by iterating through the destination string looking for the first null terminator. This mean, by the time you have cat-ed all 3 strings, you have done more looping than you need. A better way of concatenating strings into a single array would be to use sprintf, which would look something like this:

sprintf(fullName, "%s %s %s", firstName, middleName, surName);

The excellent thing being that as you are completely overriding fullName, you need not put ANY null terminators into the array for sprintf to work (optimisations ftw!), and you are only iterating over the array once. Bonza!

Hope I haven't scared you with any of this, I'm just trying to help you learn!

 

[Ronin Storm]

Yep, all of that is making good sense to me. Thank you both.

A bunch of the functions and detailed concepts you guys have brought up have not yet been introduced in the book and, I'm getting the impression, some still won't be by the end of the book. I now understand that the book's aim is to give a good run in on all the basics and leave you a rounded neophyte capable of picking up a more advanced text/course. Still got 8 chapters to go (out of 21) and some appendices of advanced information (e.g. linked lists). Probably unreasonable to think one can learn much of C++ from a 21 "day" course.

 

[thatbloke]

Yep, all of that is making good sense to me. Thank you both.

Probably unreasonable to think one can learn much of C++ from a 21 "day" course.

You can learn about the syntax of C++ easily in that kind of time (which you are doing). But to learn the vocabulary (i.e. how best to achieve certain aims) will take bloody ages!

But yes, www.cplusplus.com should be on your bookmarks if not already. Also, if you are using a linux workstation of some kind all of the standard c functions have man pages too which are helpful too.

 

[thatbloke]

I just thought - to expand on the stuff elD was on about regarding your fullName array, you could replace your initialisation with this and it would have the same effect, initialising it all to null:

char fullName[length] = {0,};

or if you prefer:

char fullName[length] = {'\0',};

I personally prefer the first method. But that, of course, is personal preference (it works because the integer value 0 is equal to the integer value of '\0')

The comma (as I am led to believe by my boss) tells it to set all members of the array to the same as what you put in before the comma.