views:

113

answers:

2

My program crashes when it tries to assign one object to the other, do you guy see anything wrong with this?

The variables are:

Field *fields[50];
int numOfFields;
int currentField;

The function is:

Screen& operator=(Screen &scr) {
  if (this != &scr){
    for (int i = 0; i < 50; i++)
      fields[i] = NULL;

    for (int i = 0; i < scr.numOfFields; i++) 
      fields[i] = scr.fields[i];

    numOfFields = scr.numOfFields;
    currentField = scr.currentField;
   }

  return *this;
}
A: 

You should use a std::vector instead of your dynamic array and I suspect your problem will go away very quickly.

As suggested by MadCap you should also use shared pointers as this is best practice when using pointers with a container. Also this will give you the copy semantics that you expect.

This would negate the need for this:

    for (int i = 0; i < 50; i++)
        fields[i] = NULL;

and you could replace this:

   for (int i = 0; i < scr.numOfFields; i++) 
        fields[i] = scr.fields[i];

with a std::copy, something like:

fields.clear()
std::copy(scr.fields.begin(), scr.fields.end(), fields.begin());

By doing this you will remove the possibility that the crash is occurring because of some pointer access or initialisation error.

Simple advice is; stop using dynamic arrays and raw pointers and start using std::vector and boost::shared_ptr instead doing this will help to prevent you from running into these sorts of problems.

radman
It probably won't go away but the source of the error could become apparent. There's also a chance that the error doesn't even occur here.
wheaties
I don't believe your `std::copy` will work because you don't allocate any actual objects in the destination `fields` vector after the clear. Probably `std::back_inserter` would do the trick.
Mark B
+1  A: 

One problem might be that scr.numOfFields exceeds the number of fields in your destination object.

Another problem is that, or at least it seems, you are assigning pointers to your new object. This means you will have a reference to the same location twice in the program. What happens if it gets deleted in one spot and the other doesn't know about it? When you try to access the memory you'll get a seg fault.

If you have Boost you can use their shared pointers to help avoid this : http://www.boost.org/doc/libs/1_43_0/libs/smart_ptr/smart_ptr.htm

MadcapLaugher
Madcap's first comment is likely the culprit here: you're not checking that scr is the same size as the field member before you try to assign to its elements. What you should do here is delete the memory allocated in field (and its sub-elements if those are dynamically allocated), and then reallocate a new size to fit the elements you're bringing in. Also, make sure that your assignment operator syntax is correct. Use std::vector that radman suggested if you don't want to worry about tracking memory allocation.
twokats