CODEWITHSUNDEEP
c++c++11
Narek
Narek

Reputation: 39871

Delete an object after the callback is called C++

I create a new object and set a data and a callback something like this:

class DownloadData
{
    std::function<void(int, bool)>    m_callback;
    int                               m_data;
public:
    void sendHttpRequest()
    {
         // send request with data
    }

private:
    void getHttpResponse(int responseCode)
    {
        if (responseCode == 0)
        {
               // save data
               m_callback(responseCode, true);
               delete this;
               return;
        } 

        // some processing here
        if (responseCode == 1 && some other condition here)
        {
               m_callback(responseCode, false);
               delete this;
               return;   
        }
    }
}

Now the usage - I create a new object:

if (isNeededToDownloadTheFile)
{
    DownloadData* p = new DownloadData(15, [](){});
    p->sendHttpRequest();
}

But as you can see https://isocpp.org/wiki/faq/freestore-mgmt#delete-this it is highly not desirable to make a suicide. Is there a good design pattern or an approach for this?

Upvotes: 3

Views: 1807

Answers (3)

Bartek Banachewicz
Bartek Banachewicz

Reputation: 39380

If you want to put the delete out of that function, the only way is to store the object somehow. However, this raises the ownership questions: who is the owner of the asynchronous http request that's supposed to call a callback?

In this scenario, doing the GCs job actually makes the code pretty clear. However, if you wanted to make it more adaptable to C++, I'd probably settle on a promise-like interface, similar to std::async. That way the synchronous code path makes it way easier to store the promise objects.

You asked for a code example, so there goes:

Typical approach would look like this:

{
    DownloadData* p = new DownloadData(15, [](auto data){
        print(data)
    });
    p->sendHttpRequest();
}

Once the data is available, it can be printed. However, you can look at the problem "from the other end":

{
    Future<MyData> f = DownloadData(15).getFuture();
    // now you can either
    // a) synchronously wait for the future
    // b) return it for further processing
    return f;
}

f will hold the actual value once the request actually processes. That way you can push it as if it was a regular value all the way up to the place where that value is actually needed, and wait for it there. Of course, if you consume it asynchronously, you might as well spawn another asynchronous action for that.

The implementation of the Future is something that's outside of the scope of this answer, I think, but then again numerous resources are available online. The concept of Promises and Futures isn't something specific to C++.

Upvotes: 1

Galik
Galik

Reputation: 48625

If the caller keeps a reference to the downloading object then it can erase it when the download signals it has ended:

class DownloadData
{
    // true until download stops (atomic to prevent race)
    std::atomic_bool                  m_downloading;
    int                               m_data;
    std::function<void(int, bool)>    m_callback;

public:
    DownloadData(int data, std::function<void(int, bool)> callback)
    : m_downloading(true), m_data(data), m_callback(callback) {}

    void sendHttpRequest()
    {
         // send request with data
    }

    // called asynchronously to detect dead downloads
    bool ended() const { return !m_downloading; }

private:
    void getHttpResponse(int responseCode)
    {
        if (responseCode == 0)
        {
               // save data
               m_callback(responseCode, true);
               m_downloading = false; // signal end
               return;
        }

        // some processing here
        if(responseCode == 1)
        {
               m_callback(responseCode, false);
               m_downloading = false; // signal end
               return;
        }
    }
};

Then from the caller's side:

std::vector<std::unique_ptr<DownloadData>> downloads;

// ... other code ...

if (isNeededToDownloadTheFile)
{
    // clean current downloads by deleting all those
    // whose download is ended
    downloads.erase(std::remove_if(downloads.begin(), downloads.end(),
    [](std::unique_ptr<DownloadData> const& d)
    {
        return d->ended();
    }), downloads.end());

    // store this away to keep it alive until its download ends
    downloads.push_back(std::make_unique<DownloadData>(15, [](int, bool){}));
    downloads.back()->sendHttpRequest();
}

// ... etc ...

Upvotes: 0

Rob K
Rob K

Reputation: 8926

You could put them in a vector or list, have getHttpResponse() set a flag instead of delete this when it's completed, and then have another part of the code occasionally traverse the list looking for completed requests.

That would also allow you to implement a timeout. If the request hasn't returned in a day, it's probably not going to and you should delete that object.

Upvotes: 2

Related Questions