Bryce:

I tend to prefer the name 'Edit' over 'Desktop'

This statement is not consistent with your (early) implementation in application/editor.*, where you have a SPDesktop* AND an Edit*. OTOH, it supports my notion that some of your code in ui/view/edit.* really belongs in EditWidget, as part of it it is Gtkmm specific.

mental:

It's almost always better to do a straightforward implementation and get it structured appropriately -- and _then_ factor out the appropriate abstractions, than it is to introduce an extra abstraction anticipating a future need.

Yes. That's the order of steps.

JonCruz:

So I'd probably summarize that as starting with abstraction is good, just as long as it's the proper abstraction. :-)

I fear there is no such thing. Things keep changing. But thanks for your kind mail.

ralf

Quoting Ralf Stephan <ralf@...748...>:

View ViewWidget +Desktop uses +DesktopWidget +SVGView +SVGViewWidget

               (interface)
                   ==
          (abstract base class)

This feels weird to me. Ideally (I think) Desktop/SVGView should not need to know anything about the widgets attached to them -- they are basically "model" classes.

So the knowledge would be more one-way, where the widgets know about the model class, but not vice-versa.

Where the model needs to tell the widget something, it should be handled through callbacks that the widgets themselves register.

-- Placement New

Incidentally, you should no longer use placement new to initialize members of classes which have been converted to C++:

new (&_activate_signal) sigc::signal<void>();

The compiler takes care of that stuff automatically. We were only doing that in GObject-derived classes because GObject initialization bypasses the C++ compiler's normal mechanisms.

-- GC::Anchored

Also, since SPDesktop is GC::Managed now, it's a good idea to stop anchoring GC::Anchored objects from it. Failing to do so can result in memory leaks, particularly if there are reference cycles.

(SPDesktop and Inkscape::Selection do form a reference cycle...)

Under normal circumstances this means simply neither anchoring nor releasing such objects. So you would remove this bit from SPDesktop::~SPDesktop:

if (selection) { Inkscape::GC::release(selection); selection = NULL; }

But, since you were responsible for creating the object, you do still need to release it for it for it to be freeable. You can do that at creation:

selection = Inkscape::GC::release(new Inkscape::Selection(this));

This is safe because GC::release means "let the garbage collector decide", rather than "free this object", as it would for pure refcounting. The garbage collector can see that it is still referenced by a local variable/return value.

-mental

On Wed, Sep 07, 2005 at 08:13:24PM +0200, Ralf Stephan wrote:

...

...

View ViewWidget +Desktop uses +DesktopWidget +SVGView +SVGViewWidget

               (interface)
                   ==
          (abstract base class)

This feels weird to me. Ideally (I think) Desktop/SVGView should not need to know anything about the widgets attached to them -- they are basically "model" classes.

So the knowledge would be more one-way, where the widgets know about the model class, but not vice-versa.

I wondered about the existing circularity. Some of the needed callbacks would be only used by singular classes, and where the reference to the widget itself is returned, it is used by Gtk-specific classes. In these cases, Desktop is best not involved at all.

Bryce asked:

...

Do you think we would really need both C and C++ implementations?

Maybe not, although it would make switching easy. But replace C with Mac or native Windows or ... So the abstraction is needed anyway. Nearly 10 years ago, I created and maintained the yarec package that had ncurses and Gtk UIs in one binary, so the idea is not new. However, I don't propose full Gtk and Gtkmm inkscape in one binary!

Ah, that makes sense. I didn't think about alternate widget set implementations. It's entirely possible that one day someone would come along with a cool, complete wxWidgets-based interface that we would like to hook onto, and this level of abstraction would permit that.

Btw, there is a presentation in the docs directory of the codebase I did that explains some of my ideas for the rearchitecting of the View/Desktop code. You might find it worthwhile to take a look at it.

Bryce

On Sep 7, 2005, at 5:39 PM, MenTaLguY wrote:

It's almost always better to do a straightforward implementation and get it structured appropriately -- and _then_ factor out the appropriate abstractions, than it is to introduce an extra abstraction anticipating a future need.

At first I disagreed with this generalization, but then I realized some details that differentiate what I was thinking of and how it diverges.

* Adding extra abstraction for later -- more often not good.

* Starting with abstraction and implementing behind it.

The latter is what I'd suggest is good. However, the driving force would be in analyzing the base concepts involved and proceeding from there. That is, toss around all the ideas involved, and then boil them down to their pure "thing-ness" that make those things what they are. Then group up in nice modules, and abstract things at the boundaries. Then go in and implement each module.

So it would be the case for abstracting at module boundaries rather than abstracting for possible future need. Of course, when done properly the former can make the latter much easier. Then again abstracting at module boundaries helps reduce spaghetti linkage, improve testability and clarify desgin. So that can go in as an immediate payoff as compared to the "anticipating a future need" guess at payoff.

So I'd probably summarize that as starting with abstraction is good, just as long as it's the proper abstraction. :-)