ARMmbed / core-util

DEPRECATED: Mbed 3 utilities library
Other
12 stars 17 forks source link

Added referenced counted shared pointer class. #30

Closed marcuschangarm closed 8 years ago

marcuschangarm commented 8 years ago

see https://github.com/ARMmbed/core-util/pull/26

hugovincent commented 8 years ago

(Continuing discussion from other #26) My view is that if it's almost-but-not-quite shared_ptr we're probably doing it wrong. If it's exactly the same except in a different namespace, then we can stand on the shoulders of all the documentation/tutorials/etc that exists out there in the world for shared_ptr. How different is it to the std:: one other than in name? You mentioned a constructor difference too. Is it complete or a subset of std::shared_ptr? thanks

hugovincent commented 8 years ago

Having said that, having this in here is awesome!!! so please do persevere :-)

0xc0170 commented 8 years ago

My view is that if it's almost-but-not-quite shared_ptr we're probably doing it wrong.

@marcuschangarm how smart is this shared pointer? I don't see an ownership of the object (there's a pointer, no destroy when counter gets 0 for example), no atomicity. This feels like "empty" shared_ptr with minimal subset of functionality.

It's always helpful if you can provide a test case with the implementation, where we could see the usage of this class.

marcuschangarm commented 8 years ago

@hugovincent

We are not using C++11, we are already doing it wrong! :D

Which is also the answer to your question. The shared_ptr constructor uses variadic templates. I wanted users to be conscious about this hence the different name.

The SharedPointer class has all member functions that doesn't depend on variadic templates. It is missing most of the non-member functions.

Edit: It is when you use the make_shared constructor that variadic templates are being used.

marcuschangarm commented 8 years ago

@0xc0170

The destruction part happens in line 211.

I'm not sure about the atomicity. It was originally safe to be used from interrupt context, but I removed it because should you even be using shared pointers when handling interrupts?

Here is an example from the menu framework on the watch. Objects are being passed around as SharedPointers so they can be moved, cached, and destroyed regardless of origin.

/* Function being called when user presses the button with a menu selection highlighted. 
*/
SharedPointer<UIView::Action> GenericMenuUI::getAction()
{
    /* Find selected cell index */
    unsigned int index = UITableKineticView::getMiddleIndex();

    /* Get object related to calling action on highlighted cell. */
    SharedPointer<UIView::Action> returnObject = table->actionAtIndex(index);

    /*  If the returned object is a table itself, use it to create a new menu object
        and use that as a return value instead. 
        If the returned object is not recognized, return it to caller.
    */
    if (returnObject->getType() == UIView::Action::Array)
    {
        SharedPointer<UIView::Array> _table = returnObject->getArray();

        /* create new table based on data array and touch slider */
        SharedPointer<UIView> menu(new GenericMenuUI(_table, slider));

        /* create new Action object based on the new table */
        returnObject = SharedPointer<UIView::Action>(new UIView::Action(menu));
    }

    return returnObject;
}
bogdanm commented 8 years ago

This PR should have a suitable test associated with it, testing things like:

See the test/ directory for some examples.

marcuschangarm commented 8 years ago

@bogdanm @0xc0170 @hugovincent

Uploaded test application and incorporated (all?) the changes Bogdan mentioned.

I've put it in mbed::util like the other utilities, but shouldn't it be core::util?

bogdanm commented 8 years ago

I've put it in mbed::util like the other utilities, but shouldn't it be core::util?

We've discussed that and decided to keep all the 'core' components under the mbed namespace.

bogdanm commented 8 years ago

+1. Nice work!