problem with copy ctor / operator=

[gcampton]

My program has been working find, until I implemented a copy ctor, and operator=
I changed around a couple vars, to make the scope of the class function wide, but while being pretty much just a declaration.
and then went to assign and/or copy in my implementation.

basically what I mean is:

Array2D nullArray();  // function scope
{
   {
      {
         {  // deep nested initialization. scope in here only
            Array2D newArrayWithValues( 45, 32 ); 
            nullArray(newArrayWithValues);
         }
      }
   }
}

Before this I was simply sending to a function straight away, but that gave me logic errors. So made a copy ctor, operator=, and default ctor...
Now I have a ton of errors. Class is as follows:

/**
 * array2d.h
 * Using Array2D class:
 * 		Array2D chArr(row, col);
 * 		chArr(1,2) = 'A';
 */

#ifndef ARRAY2D_H_
#define ARRAY2D_H_

#include <new>

class Array2D
{
  public:
	Array2D() : _width(0), _height(0), _array(0)
	{ }
	Array2D(unsigned wd, unsigned ht) : _width(wd), _height(ht), _array(0)
	{
		if (wd > 0 && ht > 0) _array = new (std::nothrow) char[wd * ht];
	}
	virtual ~Array2D()
	{
		delete[] _array;
	}

	// arrays stored linearly: conceptually stored as table
	const char& operator() (unsigned x,unsigned y) const
	{
		return _array[ y*_width + x ];
	}
	char& operator() (unsigned x,unsigned y)
	{
		return _array[ y*_width + x ];
	}

	// get dimensions
	unsigned getWd() const { return _width; }
	unsigned getHt() const { return _height; }

  private:
	unsigned _width;
	unsigned _height;
	char* _array;

	// copy // assignment //
	Array2D(const Array2D& cp)
	: _width(cp._width), _height(cp._height), _array(0)
	{
		if (cp._width > 0 && cp._height > 0) this->_array =
				new (std::nothrow) char[cp._width * cp._height];

		for (unsigned z=0; z<(cp._width*cp._height); z++)
			this->_array[z] = cp._array[z];
	}
	Array2D& operator = (const Array2D& cp)
	{
		this->_width=cp._width;
		this->_height=cp._height;

		if (cp._width > 0 && cp._height > 0) this->_array =
				new (std::nothrow) char[cp._width * cp._height];

		for (unsigned z=0; z<(cp._width*cp._height); z++)
			this->_array[z] = cp._array[z];

		return *this;
	}
};

#endif /* ARRAY2D_H_ */ 

Errors:

../a1.cpp: In function `void processFile(std::string)':
../a1.cpp:167: error: request for member `getWd' in `puz_arr', which is of non-class type `Array2D ()()'
../a1.cpp:169: error: assignment of function `Array2D puz_arr()'
../a1.cpp:169: error: cannot convert `Array2D' to `Array2D ()()' in assignment
../a1.cpp:90: error: too many arguments to function `Array2D puz_arr()'
../a1.cpp:185: error: at this point in file
../a1.cpp:185: error: no match for 'operator=' in 'puz_arr()() = (&tmp_puz_line)->std::basic_string<_CharT, _Traits, _Alloc>::operator[] [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>](((unsigned int)i))'
../array2d.h:68: note: candidates are: Array2D& Array2D::operator=(const Array2D&)
../a1.cpp:206: error: invalid initialization of non-const reference of type 'Array2D&' from a temporary of type 'Array2D (*)()'
../a1.cpp:36: error: in passing argument 1 of `void processPuzzle(Array2D&, Array2D&, std::string, int, int)'
../a1.cpp:90: error: too many arguments to function `Array2D puz_arr()'
../a1.cpp:216: error: at this point in file
../a1.cpp:216: error: no match for 'operator<<' in 'std::cout << puz_arr()()'
/usr/lib/gcc/i686-pc-cygwin/3.4.4/include/c++/bits/ostream.tcc:63: note: candidates are: std::basic_ostream<_CharT, _Traits>& std::basic_ostream<_CharT, _Traits>::operator<<(std::basic_ostream<_CharT, _Traits>&(*)(std::basic_ostream<_CharT, _Traits>&)) [with _CharT = char, _Traits = std::char_traits<char>]
/usr/lib/gcc/i686-pc-cygwin/3.4.4/include/c++/bits/ostream.tcc:74: note:  std::basic_ostream<_CharT, _Traits>& std::basic_ostream<_CharT, _Traits>::operator<<(std::basic_ios<_CharT, _Traits>&(*)(std::basic_ios<_CharT, _Traits>&)) [with _CharT = char, _Traits = std::char_traits<char>]

the only lines i changed in my .cpp were:

	Array2D puz_arr();
	Array2D solved_arr();  // these are the function wide variables. 

if (! createdArr)
{
   Array2D new_puz(row,col);
   Array2D new_solved(row,col);
   createdArr = true;

   if (puz_arr.getWd() == 0) // this gives error....
   {
      ; 
   }
}

[Disch]

I only skimmed, but here's what I spotted:

1) Don't put parenthesis when calling the default ctor like that.

Array2D puz_arr();  // bad.  This is a function prototype.
   // it declares a function named 'puz_arr' that takes no parameters and returns a ClassName type

Array2D puz_arr;  // good.  This calls the default ctor. 

2) Your rows/cols look like they're backwards. The way Array2D takes it's parameters in the ctor is (width,height). IE: (cols,rows) ... (x,y). In the comment at the top (and in the way you create new_puz and new_solved in your code below), you're doing it the opposite way.

3) your assignment operator and copy ctor are private and therefore inaccassable.

4) You cannot call the copy ctor after an object has already been constructed. Objects are only constructed once!

Array2D nullArray;  // nullArray is constructed here
{
   {
      {
         {
            Array2D newArrayWithValues( 45, 32 ); 
            nullArray(newArrayWithValues);  // can't construct it again!!
         }
      }
   }
}

Alternatives are to resize the array (or assign it to a different size), or to create the array with new:

Array2D* nullArray;
{
  nullArray = new Array2D( 45, 32 );
}

// but then you have to delete it
delete nullArray;

[PanGalactic]

Does changing 90 & 91 to this help?

	Array2D puz_arr;
	Array2D solved_arr;  // these are the function wide variables. 

[PanGalactic]

BTW, how do you change the numbering scheme in code tags?

[Disch]

Does changing 90 & 91 to this help?

It should.

But you tell me. Did that fix your errors? =P

BTW, how do you change the numbering scheme in code tags?

afaik, you can't.

[PanGalactic]

@Disch: Look at the OP -- the last two code fragments start at lines 90 & 161 respectively.

[gcampton]

@PanGalactic
[code firstline=99]

@Disch
Yep that was it, silly mistake.. that always gets me switching between Java and C++.

[Disch]

oh wow. Neat @ the code tags

[gcampton]

Ahhh see I think that was my problem right from the beginning, I was looking at 2D arrays differently to you.

[rows] [cols]

Because you have a 1D array say ints: int myarr[ 13 ];
this is an array with 13 cols.

when you create a 2D array, it's an array of arrays [ 13 ] cols wide.

so: int myarr [5] [13];

5 rows (arrays) of 13 columns (array)....

Fundamentally with this class I don't think it makes any difference. it's only when you go to output the way in which you output is important.

[gcampton]

Now saying that and being I have written most of my .cpp as (row, cols) I changed the class to reflect this:
But trying to get my head around the operator() now I've gone and changed it around....
should it be:

const char& operator() (unsigned x, unsigned y) const
{
   return _array[ y*_height + x ];
}

or because I'm looking at it as x=row=height, y=cols=width

[codefirstline=39]
const char& operator() (unsigned x, unsigned y) const
{
return _array[ x*_height + y ];
}
[/code]

or do I just need to switch the params x, y to y, x...
omg my head wants to explode....

never mind it's only the naming I've changed so it should be the first...

[gcampton]

something's going wrong, with a 4*7 table:

size=4,7
searchfor=and

fandnad
aghiank
tnxcadv
wpdhjyu

it seems to be printing memory locations not in the array. without causing a stackdump or anything.

[Disch]

you probably want this:

// return char or const char& -- whatever
char operator() (unsigned y, unsigned x) const
{
    return _array[ (y*_width) + x ];
}

Read-like-a-book order is always (y*width)+x.

Only do (x*height)+y if you want "top down" arrangement.

Of course it doesn't really matter I guess.

Just don't do x*width or y*height because that makes no sense.


EDIT:

Re your new post:

Can you post your latest code?

[gcampton]

Well that fixed the first 2 puzzles 4*7 and 7*4 but then a 5*5 spurted out gibberish which makes no sense...
and a few lines down caused a stackdump:

/**
 * array2d.h
 */

#ifndef ARRAY2D_H_
#define ARRAY2D_H_

#include <new>

class Array2D
{
  public:
	Array2D() : _height(0), _width(0), _array(0)
	{ }
	Array2D(unsigned ht, unsigned wd) : _height(ht), _width(wd), _array(0)
	{
		if (wd > 0 && ht > 0) _array = new (std::nothrow) char[ht * wd];
	}
	virtual ~Array2D()
	{
		delete[] _array;
	}

	// arrays stored linearly: conceptually stored as table
	const char& operator() (unsigned x,unsigned y) const
	{
		return _array[ y*_width + x ];
	}
	char& operator() (unsigned x,unsigned y)
	{
		return _array[ y*_width + x ];
	}
	// copy // assignment //
	Array2D(const Array2D& cp)
	: _height(cp._height), _width(cp._width),  _array(0)
	{
		if (cp._width > 0 && cp._height > 0) this->_array =
				new (std::nothrow) char[ cp._height * cp._width ];

		for (unsigned z=0; z<(cp._height*cp._width); z++)
			this->_array[z] = cp._array[z];
	}
	Array2D& operator = (const Array2D& cp)
	{
		this->_height=cp._height;
		this->_width=cp._width;

		if (cp._width > 0 && cp._height > 0) this->_array =
				new (std::nothrow) char[cp._height * cp._width];

		for (unsigned z=0; z<(cp._height*cp._width); z++)
			this->_array[z] = cp._array[z];

		return *this;
	}
	// get dimensions
	unsigned getWd() const { return _width; }
	unsigned getHt() const { return _height; }

  private:
	unsigned _height;
	unsigned _width;
	char* _array;
};

#endif /* ARRAY2D_H_ */ 

I'd post the .cpp but I pass the post char limit by about 4k :P

[Disch]

Problems I see:

1) Your assignment operator is causing a memory leak. You are reallocating _array without deleting the original array.

2) You don't protect against self assignment, which could be a problem:

Array2D& operator = (const Array2D& cp)
{
    if(&cp == this)
        return *this;

    // the rest of the function here
}

3) You still seem to be doing (x,y) -- I thought you were switching to (y,x)?

4) Not a problem.. but any particular reason you're using the nothrow version of new? It's fine that you are, I'm just curious.


#3 is the only one I can see causing a bad access, but it wouldn't be a problem with a 5x5 array because the dims would be the same.

Other than that, though, it all looks good to me.

EDITS:

Could it be you're stepping out of bounds somewhere? (in the cpp file, I mean -- I don't see any bad accesses in this class)

You could check this by putting some bounds checking in your () operator temporarily.

[gcampton]

1 2 fixed
3. I failed geometry :)
4. the teacher wants us to, I'm guessing I should be checking for nulls somewhere...

3. I swapped the params only, this seems to have fixed it.

now I just need to get some decent output and delete all these debugging lines.

with this section of code, do the newObjects destroy themselves at the end of the if block?

if (! createdArr)
{
   Array2D new_puz(row,col);
   Array2D new_solved(row,col);
   createdArr = true;

   puz_arr = new_puz;
}

[Zhuge]

Yes, they go out of scope there.

[gcampton]

Thanks everyone, I think I'm done, with only a couple hours to spare :s still got to write a report yet *cry*

[gcampton]

I lied, ANSI hates me... just tried to compile with -pedantic -ansi under a shell, which I thought eclipse would do anyway.. pretty sure i told it to.
and:

void processPuzzle(Array2D& puz_arr, Array2D& solved_puz, string word, int row, int col)
{
   int arr_word_size = word.size();
   char arr_word[word.size()];

ISO C++ forbids variable-size array

But I have to convert to char array for processing because I'm not allowed to use cstring or string class to process puzzle. How can I get around this...

char arr_word[arr_word_size];
and const int arr_word_size = word.size();

combinations don't work either...


edit:
urgh got it,
char* arr_word = new char[word.size()];

[Disch]

Can't use string? Why not? Some weird assignment limitation?

Can you use vector? You could just make a vector of chars.

If not you can dynamically allocate:

char* arr_word = new char[word.size()];

// do stuff with arr_word here

delete[] arr_word;

EDIT: too slow. Guess you figured it out on your own. Heh =)

[gcampton]

Some weird assignment limitation?

yep, as if it wasn't hard enough already.
my logic didn't change the program in the slightest, I mean if I was using string, my code would fundamentally be almost exactly the same except for converting string to a dynamic char * and using that to search the puzzle.

In regards to vector they are pretty strict in regards to following a set of rules put in place by industry professionals. Basically they have to follow a set of guidelines for teaching that employers want out their perspective employees. what they think a graduate should be able to do.
So yea, no STL.