Question

Often I add an Empty method to my C++ objects to clear the internal state using code similar to the following.

class Foo
{
private:
    int n_;
    std::string str_;
public:
    Foo() : n_(1234), str_("Hello, world!")
    {
    }

    void Empty()
    {
        *this = Foo();
    }
};

This seems to be better than duplicating code in the constructor, but I wondered if *this = Foo() is a common approach when wanting to clear an object? Are there any problems with this waiting to bite me on the backside? Are there any other better ways to achieve this sort of thing?

Was it helpful?

Solution

I'd let the constructor call my function instead:

class Foo
{
private:
    int n_;
    std::string str_;
public:
    Foo()
    {
        Reset();
    }

    void Reset()
    {
        n_ = 1234;
        str_ = "Hello, world!";
    }
};

Yes, you're unnecessarily initializing the string as an empty string first, then doing an assignment, but this is much clearer.

OTHER TIPS

Potential problems? How do you know that *this really is a Foo?

What you're doing with this Empty method, is essentially the same as manually assigning a newly constructed object to a variable (the thing that the Empty function does).

Personally, I'd remove the Empty method, and replace all uses of that method with this:

// let's say, that you have variables foo and pfoo - they are properly initialized.
Foo foo, *pfoo;

// replace line "foo.Empty()" with:
foo = Foo();

// replace line "pfoo->Empty()" with:
delete pfoo;
pfoo = new Foo();
// or
*pfoo = Foo();

I really see no advantage in having this Empty method. It hides what is really happening to the object on witch it's called, the name isn't the best choice either.

If the caller really wants a clean object - he will have no problem constructing the object himself.

Also, consider making objects immutable, i.e., when constructed, they cannot be changed. This can in a lot of scenarios save yourself from unanticipated side effects.

There is something even more common than what you have suggested. using swap.

Basically you do something like this:

T().swap(*this);

since many standard containers (all of the STL containers?) have a constant time swap method, this is a nice and simple way to clear a container and make sure it's storage is released.

Similarly, it is a good way to "shrink to fit" a container as well, but using the copy constructor instead of the default constructor.

consider using placement new:

void Empty() {
    this->~Foo();
    new (this) Foo();
}

Your code invokes operator = which might lead to all kinds of side-effects.

EDIT in response to the comments. – This code is definitely well-defined, the standard explicitly allows it. I'm going to post the paragraph later if I find the time. About delete – of course. What I meant was ~Foo(), this was an oversight. And yes, Rob is right as well; destructing the object is actually necessary here to call the string's destructor.

This can be a potential source of memory leak if you have a dynamically allocated memory in the constructor.

Here's how i do it:

class Foo {
private:
    int n_;
    std::string str_;
public:
    Foo() : n_(1234), str_("Hello, world!")
    {
    }

    void Empty()
    {
        Foo f;
        swap(f);
    }

    void swap(Foo & other) {
        std::swap(n_, other.n_);
        swap(str_, other.str_);
    }
};

void swap(Foo & one, Foo & other) {
    one.swap(other);
}

Put the swap function into the same namespace like the Foo class. Argument dependent lookup finds it when users do a call to swap to swap two Foo. You can implement operator= using your swap function too.

This seems to be better than duplicating code in the constructor, but I wondered if *this = Foo() is a common approach when wanting to clear an object?

Clearing an object isn't that common a thing to do: more commonly, either an object (perhaps even an immutable object) is instantiated and contains real data, or it isn't instantiated.

The most common kind of thing that are reset would be containers ... but, you wouldn't be writing your own container classes now, would you.

Are there any problems with this waiting to bite me on the backside?

Yes:

  • If this isn't really a Foo but is instead a DerivedFoo
  • If Foo's assignment operator doesn't exist, or if it's buggy (e.g. if it's not defined and the default operator isn't good, e.g. because of a data member is a naked pointer).

Are there any other better ways to achieve this sort of thing?

Yes, maybe a free function would be better (would avoid both the above problems):

template<class T> void reconstruct(T* p)
{
    p->~T();
    new (p) T();
}

Yes, this is not efficient in terms of performance (creating another foo object instead of working in place) and it will bite you if you'd allocate memory in the constructor with a nasty memory leak.

Making it safer in terms of memory would be calling the this->delete and this = new foo() - but it will be SLOW.

if you want to be superfast create a static empty object field and memcpy it in Reset.

if you want to be just fast assign the properties one by one.

if you want to maintain reasonable style without duplication call Reset from Ctor as Ates Goral suggested but you'll lose the faster construction with the default params.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top