GNOME.org
 

Re: Replace Gtk::manage() with std::unique_ptr<>?
Correct me if I'm wrong, but doesn't Gtk::manage work more like a shared pointer? i.e. maintain a count of the references to the object, deleting the object pointed to when the ref count goes to zero.
As you've said, using unique_pointer mandates a design with only one access point to the controlled object, and there's plenty of interesting things you may want to do to widgets after construction. We use Gtk::manage() freely because it keeps track of widget memory use for us, but doesn't control access to it within the code.
One option might be to provide 2 variants for Gtk::manage(); one using shared_pointer semantics and one using unique_pointer, with a default to shared (because it'd be backwards compatible). 
It's also considered wise to receive a std::unique_ptr as a parameter
if the method really plans to take ownership. For instance:
   void Foo::take_thing(std::shared_ptr<Thing> thing);
I think you've typed std::shared_ptr<Thing> when you meant std::unique_ptr<Thing> ? 

Ian.

On 07/02/16 10:26, Murray Cumming wrote:

One thing about this bothers me:

Unlike a call to Gtk::manage(), a call to std::move() suggests that we
shouldn't touch the std::unique_ptr<Widget> again. But we have some API
that seems to require that. For instance, we have code in demowindow.cc
that would end up a little like this:

   auto widget = std::make_unique<Button>("some button");
   notebook.append_page(std::move(widget), "something");
   notebook.child_property_tab_expand(*widget) = true;

That would work, but it's scary to use widget after we've called
std::move() on it. This doesn't seem much better:

   auto widget = std::make_unique<Button>("some button");
   Gtk::Widget* widget_to_expand = *widget;
   notebook.append_page(std::move(widget), "something";
notebook.child_property_tab_expand(*widget_to_expand) = true; 


It would be difficult to change our API to make it unnecessary to ever refer to a child Widget* after we've 
added it to a container.

Murray

On Fri, 2016-02-05 at 14:47 +0100, Murray Cumming wrote:

The trend in modern C++ is to use std::unique_ptr<> by default to
express ownership, usually via std::make_unique() (in C++14), insead
of
using a raw pointer via a "naked new".

So, unless you know something else is necessary, this would be good:
   auto thing = std::make_unique<Thing>();
instead of this:
   auto thing = new Thing();
or
   Thing* thing = new Thing();



So I was wondering if we could use this idea instead of
Gtk::manage(),
which has much the same sense of "take ownership", and eventually
deprecate Gtk::manage().

Then we could do this, for instance:
   auto button = std::make_unique<Gtk::Button>("a button");
   button->show();
   container.add(std::move(button));
instead of this:
   auto button = Gtk::manage(new Gtk::Button>("a button"));
   button->show();
   container.add(*button);

This would work too, I think:
   container.add(std::make_unique<Gtk::Button>("a button"));


Then we would be using standard C++ syntax/API instead of custom
gtkmm
API.


However, it would need us to add overloads for methods that currently
take Widget& parameters. But I think that's doable. For instance:

void Container::add(std::unique_ptr<Widget> widget)
{
   add(*(Gtk::manage(widget.release())));
}


Thoughts?

--
Murray Cumming
murrayc murrayc com
www.murrayc.com
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]