gtk-sharp icon indicating copy to clipboard operation
gtk-sharp copied to clipboard

generator: Overhaul GInterface type naming approach

Open knocte opened this issue 10 years ago • 11 comments

In the past, for the GInterface Foo we used to have: a) FooAdapter: the helper class to use GInterface implementations. b) Foo: the interface with the methods from native side. c) FooImplementor: the interface to inherit from .NET classes.

Learning about these special suffixes "Adapter" and "Implementor" was a bit cumbersome and not very API-consumer friendly.

Some months ago we modified gtk-sharp's generator to follow the "I" prefix convention for .NET interfaces, which leaves us as:

a) FooAdapter. b) IFoo. c) IFooImplementor.

This is good because now the name "Foo" is free, so we can rename "FooAdapter" to "Foo", and there's one less thing to learn (so this is one of the things this commit achieves).

The next step is, then, getting rid of the "Implementor" suffix, how? Simply switching the way this is exposed, as:

b) IFoo -> IFooBase c) IFooImplementor -> IFoo

This way, if one wants to create a .NET class that implements the GInterface Foo, she simply needs to inherit from "IFoo". And when consuming, from .NET, native classes that implement the Foo GInterface, the interface they will implement will be IFooBase instead of IFoo (but there's no real need to learn about this latter suffix, as by using the API you will simply be exposed to it).

Discussed with and reviewed by: Stephan Sundermann <[email protected]>

knocte avatar Aug 05 '14 13:08 knocte

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

monoadmin avatar Aug 05 '14 13:08 monoadmin

This would prevent also some "head scratching" when people in the past inherited from IFoo and saw that their class was not working.

knocte avatar Aug 05 '14 13:08 knocte

Love this, having IFoo and Foo is great! Not sure about the IFooBase naming, though... what's it the Base of? Base is usually used to mark abstract or base classes that provide basic functionality, not really interfaces, and it's a really generic name. Maybe IFooNative?

shana avatar Aug 06 '14 14:08 shana

Most of the time, IFooBase is a super-set of methods from IFoo, containing the virtual methods that you can override (via inheritting from IFoo in .NET, or by privately overriding in C in GObject), plus the methods that the GInterface provides with a "base" implementation (like a Mixin) which AFAIK cannot be overriden. That's why I used the word "Base", but certainly any other suggestions are welcome (Native suffix is interesting, maybe better indeed, although I have the feeling that someone will come here soon with an even better suggestion :) ).

knocte avatar Aug 06 '14 14:08 knocte

I agree that it's not a very intuitive API and that it requires reading some docs and/or code samples before being able to use it. But I'm not sure, if this can be solved by changing the naming, because in my opinion inheriting from any IFoo (current naming) is comparatively complicated - and the current naming reflects that (it doesn't make it worse). I think, the easiest way to improve the current system is to improve documentation about it. In my opinion the names FooAdapter and IFooImplementor are actually well chosen, because FooAdapter is something, you have to plug in, for you GInterface implementation to work, thus the name "Adapter". IFooImplementor represents the portion of IFoo that you "have to implement" to make it work. (The only names that may better fit, were "IFooImplementation" or "IFooImpl", because the class that inherits from that interface is not an "implementor" (that would be the programmer), but an implementation.) IFoo is the main component of the three and it represents the thing that we actually are consuming in other classes of the library. For example, a Gtk.TreeView has an ITreeModel as its Model property. If you'd call the type ITreeModelBase or ITreeModelNative, it wouldn't fit as good as ITreeModel. If we'd look at Gtk as a domain model, renaming ITreeModel to e.g. ITreeModelNative would be like suffixing an entity name with a technical term, as if it were some technical implementation detail, not The actual thing. Furthermore, if someone consults C API docs or other language binding docs to learn about Gtk.TreeView.Model, he or she would be confused by the different type names.

About the name suggestions: IFooBase is a misleading name in my opinion, because it suggests there is an inheritance relationship between IFoo and IFooBase, which actually isn't. If there were some kind of conceptual inheritance between those two, it would be the other way around, because IFooBase implements a super-set of methods from IFoo, which is indicative for a subclass not a base class. IFooNative is better, because it doesn't sound like inheritance. However, it still suffers from the problems described in above.

riccioclista avatar Sep 08 '14 21:09 riccioclista

Hello! I'm the build bot for the Mono project. I need approval from a Mono team member to build this pull request. A team member should reply with "approve" to approve a build of this pull request, "whitelist" to whitelist this and all future pull requests from this contributor, or "build" to explicitly request a build, even if one has already been done. Contributors can ignore this message.

monoadmin avatar Sep 08 '14 21:09 monoadmin

Thanks everyone for this proposal and for your comments.

I'd tend to be of the same opinion as Antonius, so in favor of keeping things as-is.

  • IFoo is the "naked", simple interface that you use just to work with the thing.
  • IFooImplementor represents the contract for implementors of IFoo

My guess would be that IFoo is currently used more often than IFooImplementor, so better keep the more frequent form as straightforward as possible.

Furthermore, after we apply this, I think it would be more confusing for people porting their GTK 2 code: if they were using TreeModel with GTK 2, they should now change it to ITreeModel, even if a TreeModel class still exists.

So I think the pain inflicted on existing code is not justified by the potential gains from this naming change.

I'm keeping this open for now, maybe I can change my mind.

bl8 avatar Sep 21 '14 14:09 bl8

Thanks for keeping it open, because I have some more ideas in my head that I would like to explain carefully in the coming days/weeks, which maybe are better suited.

knocte avatar Sep 22 '14 11:09 knocte

After a bit less than a year, I've come back to this PR :)

I have two questions:

  1. Would a smaller PR, that just renames FooAdapter to Foo, be accepted? I think it has value on its own.

  2. I like Antonious' suggestion about renaming "Implementor" to "Implementation". Maybe the best approach here is let IFoo interface not actually exist, and have two different suffixes (so when discoverying the API via intellisense, it's easy to distinguish one from another). So how about IFooImplementor -> IFooImplementation, and IFoo -> IFooConsumable?

knocte avatar May 25 '15 19:05 knocte

@knocte: ad 1) In my opinion, this would be a great idea, if we also were to change the adapter class from a regular class to an abstract one. The reason is that it would force the programmer to create a cleaner API. The API used by users of the subclass would then only consist of the consumable interface IFoo and the subclass MyFoo (for instantiation). With the current model the subclass API user has to deal with 3 classes: IFoo, FooAdapter and MyFooImplementor, which is not that straight forward to understand because you have to know about the mechanics of GInterface implementation. To illustrate the above, if the adapter class (Foo) were an abstract class, the subclass programmer would have to do this:

public class MyFoo : Foo
{
    public MyFoo ()
        : base (new MyFooImplementor ())
    {
    }
}

public class MyFooImplementor : IFooImplementor
{
}

And the subclass consumer would use it like this:

IFoo myFoo = new MyFoo ();

With the current model the subclass consumer writes code like this:

var myFoo = new FooAdapter (new MyFooImplementor ());

ad 2) As I said before, it certainly would be possible to use "IFooImplementation" instead of "IFooImplementor". I'm just not sure whether the gained value is enough to merit the change. About "IFooConsumable": I don't think this is a good idea, because subclass users don't have to know about the consumable interface concept; they just want to use IFoo.

riccioclista avatar May 30 '15 15:05 riccioclista

With all that I wrote in the previous comment, I still think its best to leave things naming-wise as they are, or to change the way GInterfaces are exposed entirely. If it is possible (from a code generation point of view), the easiest to use C# model to expose GInterfaces would be:

public interface IFoo
{
}

public abstract class Foo : IFoo
{
}

with Foo being the adapter and implementor base class in one. There are 2 things to consider:

  • The abstract Foo class exposes the implementable interface portion as abstract members
  • Multiple inheritance is still supported via the IFoo interface

This, however, is totally outside the scope of this PR.

riccioclista avatar Jun 04 '15 09:06 riccioclista