GNOME Bugzilla – Bug 556145
Move child API into GooCanvasContainer
Last modified: 2021-05-17 13:39:04 UTC
I think I've had a conversation about this on a mailing list, but I can't find it now, and I don't remember a good answer. Sorry if I'm wrong. I think the child API (get_child(), etc) should be moved into the GooCanvasContainer, instead of the GooCanvasItem, because that obviously makes more sense. Openismus is likely to provide a patch for that.
Yes, I agreed a container class would have been better in hindsight. But it would probably break API so wouldn't happen for a while.
I've added this to our list of mini tasks for people at Openismus so a patch can be ready for whenever we want it.
When you say GooCanvasContainer, do you mean GooCanvasGroup here? Or, do you mean to create a new GooCanvasContainer interface, that has GooCanvasItem as prerequisite and is implemented by GooCanvasGroup. I think I'd prefer the second option for cleanliness, but I'm not sure what you had in mind.
I don't understand the need for a GooCanvasContainer. What item would be a container but not a group?
I don't know whether there is an actual need for this. I was only wondering out of consistency reasons since GooCanvasItem is an interface as well. For example, I don't think there is an Item that does not derive from GooCanvasItemSimple, either. Also, it's not possible to create a container that does not derive from GooCanvasItemSimple (via GooCanvasGroup) that way. Again, I'm not sure whether that's needed or how much a restriction it is.
I started to implement this, and I really think that a separate GooCanvasContainer interface would fit better into the current GooCanvas design, even if there is no immediate need for it. Damon, have you any preference on this?
Armin, could you explain why?
Since GooCanvasItem is an interface, it should not have anything to do with an implementation of it. However, GooCanvasGroup implements GooCanvasItem, by deriving from GooCanvasItemSimple. On the one hand this means that methods such as goo_canvas_item_get_parent need to "know" GooCanvasGroup, which seems wrong, (the idea of an interface is to be generic, not relying that a specific implementation exists). And on the other hand this means that every item that wants to be a container needs to derive from GooCanvasItemSimple as well (which is not the case for non-container items). So this would remove the flexibility to create container items that do not derive from GooCanvasItemSimple. Again, it's not a technical issue. It's more a question of design or consistency.
OK. I guess I don't know why GooCanvasItem is an interface, but that's a separate issue.
Created attachment 122041 [details] [review] Initial patch This patch introduces a GooCanvasContainer interface, moves all the relevant functionality from GooCanvasItem to GooCanvasContainer and adapts the rest of the library and the demo for the API changes.
Damon, are you still against the idea of an API break in goocanvas? I don't think it would disturb anybody much because they can keep on using the old API.
I'd only want to break the API to add a major new feature. I don't really think this is important enough.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/goocanvas/-/issues/16.