How to swap array using pointer when array is a data member of a class

Question

I am trying to swap the content in the arrays by swapping the pointers pointing to the two arrays.

My method is the same as what Daniel answered in this question: Swap arrays by using pointers in C++. But the difference is that my array will be a member in a class. My code can be compiled successfully, but the output results are quite weird.

This is my header file:

#include <stdio.h>
#include <iostream>

class Map
{
public:
    Map(int times); // Create an empty map (i.e., one with no key/value pairs)

    int size();     // Return the number of key/value pairs in the map.
    void dump();
    void swap(Map &other);
    int *retrieve();
    void setptr(int *newptr);

private:
    int *ptr;
    int array_1[5];
};

Here is my implementation:

#include "Map.h"
#include <iostream>

using namespace std; 

Map::Map(int times) {
    for (int i = 0; i < 5; i++) {
        array_1[i]=i*times;
    }
    ptr=array_1;
}

void Map::dump() {
    ptr=array_1;
    for (int i = 0; i < 5; i++) {
        cout << *ptr << endl;
        ptr++;
    }
    for (int i = 0; i < 5; i++) {
        ptr--;
    }  
}

void Map::swap(Map &other) {
    int *temp;
    temp = this->ptr;
    this->ptr = other.retrieve();
    other.setptr(temp);
}

int *Map::retrieve() {
    return ptr;
}

void Map::setptr(int *newptr) {
    ptr=newptr;
}

Can anyone tell me what is wrong and how to implement it smartly?

Setting a pointer to the array in the other Map means taking ownership of that array, which means you take ownership of the Map. — jxh
– jxh, Commented Jan 18, 2016 at 23:57
John, I used the dump function to output the array using ptr*. The output is 0 4528 1 1969619704 32767 — Hao9000
– Hao9000, Commented Jan 19, 2016 at 0:02
The dump function moves ptr to the end and never puts it back... — M.M
– M.M, Commented Jan 19, 2016 at 0:07

novice · Accepted Answer · 2016-01-19 07:51:40Z

0

The following code runs fine:

#include <stdio.h>
#include <iostream>
#include <conio.h>
using namespace std; 
class Map
{
public:
    Map(int times);         // Create an empty map (i.e., one with no key/value pairs)
    int size();    // Return the number of key/value pairs in the map.
    void dump();
    void swap(int &other);
    int *retrieve();
    void setptr(int *newptr);
private:
    int *ptr;
    int array_1[5];
};


Map::Map(int times){
    for (int i=0;i<5;i++){
        array_1[i]=i*times;
    }
    ptr=array_1;
}

void Map::dump(){
    for (int i=0;i<5;i++)
    {
        cout<<ptr[i]<<endl;        
    }

}

void Map::swap(int &other){
    int *temp;
    temp=this->ptr;
    this->ptr=&other;
    other = *temp;
}

int *Map::retrieve(){
    return ptr;
}

void Map::setptr(int *newptr){
    ptr=newptr;
}

int main()
{
    Map m(2);
    Map n(3);
    m.dump();
    m.swap(*n.retrieve());
    m.dump();
    getchar();
}

1) Added a main function
2) Changed Swap function But the problem that christopher pointed out will still persist i.e the pointer will point to an array in another object.

Edit: You probably need something like this:

void Map::swap(Map &other){
    Map *temp;
    temp=this;
    *this = other;
    other = *temp;
}

Map *Map::retrieve(){
    return this;
}

Note: it is probably not elegant.

edited Jan 19, 2016 at 7:51

answered Jan 19, 2016 at 0:25

novice

5454 silver badges16 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Hao9000 Over a year ago

Thank you for your code. It solved my problem. But my coding instruction requires using exactly the same signature it offers. I have to stick to void swap(Map &other).

novice Over a year ago

Is this a homework question? I think what might be required would be to swap the two Map pointers

Hao9000 Over a year ago

Yes, it is homework. Here are the requirements. void swap(Map& other); // Exchange the contents of this map with the other one. The swap function is easily implementable without creating any additional array or additional Map. Do you think swap the Map pointers would be a better solution?

novice Over a year ago

Yes, of course otherwise a pointer in one map would point to an array in another map. you have to swap the map pointers.

novice Over a year ago

are there any other requirements? post all in the question!

|

Christophe · Accepted Answer · 2016-01-19 00:21:57Z

0

The problem with your design is that the pointer refers to an array in the same object.

Suppose you have to objects a and b. If you swap their pointers, a.ptr will point to b.array_1 which contains the data. reciprocally b.ptr will point to a.array1.

Unfortunately if one of the object -- say b -- gets destroyed (because it was a local object that goes out of scope, or for whatever reason) the pointer of the remaining object would point to an array which doesn't exist anymore. This is UB.

To solve your issue, you'd neet to allocate an array dynamically in the constructor. Get rid of array_1 completely:

Map::Map(int times){
    ptr=new int[5];  // or better define a constant to avoid hard coded sizes        
    for (int i=0;i<5;i++){
        ptr[i]=i*times;
    }
}

Note that if you use pointers, you need to ensure the invarients on it. This means that you should define also the copy constructor and the assignment operator (to avoid the ptr to be blindly copied), as well as a destructor (to delete the dynamically allocated array).

P.S.: I suppose that you are learning C++ and are not yet familiar with vectors. These would avoid all the hassles here

Edit: if you experience your problem before any object is destroyed, it's because of a bad implementation of dump(): you increment the pointer there in, so that it will no longer point to the start of the array.

void Map::dump(){
    for (int i=0;i<5;i++){
        cout<<ptr[i]<<endl;  // don't change ptr value !! 
    }
}

One simple trick to avoid such problems, is to systematically declare the member functions that are not supposed to change the state of the object as const:

class Map {
   ...
   void dump() const;
   ...
}

Then the compiler issues an error if you try to accidentally change a member.

edited Jan 19, 2016 at 0:21

answered Jan 19, 2016 at 0:06

Christophe

75.1k7 gold badges87 silver badges182 bronze badges

2 Comments

Hao9000 Over a year ago

Christophe, thank you for your suggestion. But this is a course problem and the instruction doesn't allow us to use dynamically allocated array. Besides, my problem occurred before one of the object was destructed.

Christophe Over a year ago

Ok, I see, I've edited for the other problem in your code !

Collectives™ on Stack Overflow

How to swap array using pointer when array is a data member of a class

2 Answers 2

6 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related