0

I am struggling to write (my first) swap function for a struct that contains a pointer array.

struct Foo
{
    unsigned int Count;
    int* Items;

    Foo()
    {
        Count = 0;
        Items = 0;
    }

    Foo(const Foo& foo)
    {
        Items = 0;
        swap(foo); // cannot convert argument 1 from 'const Foo' to 'Foo *'
    }

    Foo(const unsigned int itemCount)
    {
        Count = itemCount;
        Items = new int[itemCount];
        for (int i = 0; i < itemCount; i++)
        {
            Items[i] = 123;
        }
    }

    Foo& operator=(const Foo& foo)
    {
        swap(foo); // cannot convert argument 1 from 'const Foo' to 'Foo *'

        return *this;
    }

    void swap(Foo* foo)
    {
        unsigned int a(Count);
        int* b(Items);

        Count = foo->Count;
        Items = foo->Items;

        foo->Count = a;
        foo->Items = b;
    }

    ~Foo()
    {
        delete[] Items;
    }
};
  1. Can anyone please help me with the syntax?

  2. The reason I am doing this is to help me understand how this can work with pointer arrays?

  3. I have read online that it is exception safe as it doesn't allocate new memory to do it? Surely a and b are both allocated memory and can therefore fail if memory isn't available?

Edit: Based on the answer by lucacox...

void swap(Foo* foo1, Foo* foo2)
{
    unsigned int a(foo1->Count);
    int* b(foo1->Items);

    foo1->Count = foo2->Count;
    foo1->Items = foo2->Items;

    foo2->Count = a;
    foo2->Items = b;
}

Called like this...

swap(&foo, this); // cannot convert argument 1 from 'const Foo' to 'Foo *'

I still get a const convert error?

Beakie
  • 1,948
  • 3
  • 20
  • 46
  • Any particular reason you do not use an `std::vector`? – Biffen Oct 02 '14 at 12:31
  • Yes. This is an exercise into swap theory. – Beakie Oct 02 '14 at 12:32
  • @Beakie [What is the Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) –  Oct 02 '14 at 12:33
  • 1
    If `swap` takes a pointer, give it a pointer. `Foo&` is not a pointer. – Jonathan Potter Oct 02 '14 at 12:33
  • I tried swap(*foo) but I get illegal indirection – Beakie Oct 02 '14 at 12:35
  • You mean `swap(&foo)`, not `swap(*foo)`. – leemes Oct 02 '14 at 12:35
  • 1
    What you're trying to implement looks like the [copy-and-swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). – leemes Oct 02 '14 at 12:36
  • Sorry, quite right. I spent so long trying to solve it I tried everything. It errors with... cannot convert argument 1 from 'const Foo *' to 'Foo *' – Beakie Oct 02 '14 at 12:37
  • 1
    This is because you try to swap the provided reference which you should not change (hence it is const). Please read about the copy and swap idiom in my provided link. – leemes Oct 02 '14 at 12:37
  • @leemes Yup, that's right. – Beakie Oct 02 '14 at 12:37
  • This is the very bit of text that I read before embarking on this code. It confuses me. – Beakie Oct 02 '14 at 12:38
  • Then read it again. You made several errors when trying to follow the advice in the accepted answer of that question. To start with, why do you even call your `swap` in the copy constructor? Also, why use a pointer in your `swap`? Also, why not using `std::swap` in your swap? Also, why is it a member function with one parameter, instead of a (friended) free-standing function with two parameters? And this is not everything which is wrong in your code... Please read it again. – leemes Oct 02 '14 at 12:44
  • @Beakie If you're trying to implement the copy and swap idiom, you'll need to do the copy part too. e.g. by passing by value like `Foo& operator=(Foo foo)`. You also wouldn't use swap in the copy constructor, though if you implement a move constructor you might implement it in terms of swapping. – nos Oct 02 '14 at 12:53

3 Answers3

1

You may not change a constant object. So you may not apply member function swap to constant reference of an object of type Foo in the copy constructor. Moreover there is no sense to use it with the copy constructor of your class. Substitute this copy constructor

Foo(const Foo& foo)
{
    Items = 0;
    swap(foo); // cannot convert argument 1 from 'const Foo' to 'Foo *'
}

for these two ( one is the copy constructor without swap and other one is the move constructor with swap)

Foo( const Foo& foo ) : Count( foo.Count )  
{
    Items = new int[Count];
    std::memcpy( Items, foo.Items, Count * sizeof( int ) );
    // std::copy( foo.Items, foo.Items + foo.Count, Items );
}

Foo( Foo&& foo) : Count( 0 ), Items( 0 )
{
    swap(foo);
}

The same operation do with the copy assignment operator that is you should to define the copy assignment operator without using swap and the move assignment operator with swap.

Only member function swap should be declared as void swap( Foo & )

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

The conflict between Foo& operator=(const Foo& foo) that requires const-ness of foo, and swap, that actually modifies its arguments is fundamental one in c++. It even caused changing language standard, with "rvalues" and "move constructors" addition. Google "c++ move semantics" for more explanation.

Alexey Birukov
  • 1,565
  • 15
  • 22
-1

The error is caused by different parameters declaration:

Foo& operator(const Foo& foo)

declares parameter foo as a const (not modificable) reference to a Foo object. While

void swap(Foo* foo)

declares parameter foo as a pointer to a Foo object.

You cannot convert from const Foo& to Foo*, they are different types.

I suggest to implement the swap function by passing it Foo*:

void swap(Foo* foo1) {
    int count = this->Count;
    int* items = this->Items;

    this->Count = foo1->Count;
    this->Items = foo1->Items;

    foo1->Count = count;
    foo1->Items = items;
}
Lucacox
  • 195
  • 1
  • 9