Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: interface adapters #9132

Closed
mharthoorn opened this issue Feb 24, 2016 · 34 comments
Closed

Proposal: interface adapters #9132

mharthoorn opened this issue Feb 24, 2016 · 34 comments

Comments

@mharthoorn
Copy link

A frequent problem is that a class is suitable to have a certain interface, but as
a developer you don't have access to change that class, for example as a third party developer.
A classic solution would be to write an adapter class that does implement the required interface and map that to the interface. It would be hugely beneficial if we can create an adaptor without a need for separate adapter instantiation.

In the example below "adapter" and "adaptee" would be new C# keywords.
The adapter makes the adaptee class available as the targeted interface.

public interface IAnimal
{
    void Walk();
    int Legs { get; set; }
}

public class Cat
{
    public void Walk();
    public int Paws { get; set; }
}

public adapter Cat: IAnimal
{
    void Walk()
    {
        adaptee.Walk();
    }

    int Paws => adaptee.Legs;
}

void Main() 
{
    var animals = new List<IAnimal>();
    var cat = new Cat { Paws = 4 };
    animals.Add(cat);            // here's the magic
    foreach (var animal in animals)
    {
        if (animal.Legs > 1) 
        animal.Walk();
    }
}

For comparison: Haskell has the instance concept.

Interface to interface or class to class adapters might be options here too.

@DavidArno
Copy link

You describe an interesting structural type system that also provides extension methods and properties.

Whilst I'm a bit of a fan of structural typing, I think you are proposing this in the wrong place. It would require significant change to the CLR to implement, so you'd need to propose this as a CLR change first.

@alrz
Copy link
Member

alrz commented Feb 24, 2016

Related: #8127

@DerpMcDerp
Copy link

What does your proposal do with this:

  • cat as IAnimal
  • (IAnimal)cat
  • cat is IAnimal
  • IAnimal an = cat; an.GetType()
  • typeof(Cat).GetInterfaces()
  • cat.Legs

@bondsbw
Copy link

bondsbw commented Feb 25, 2016

I think this line would fail to compile:

if (animal.Paws > 1) 

because IAnimal has no member named Paws.

@bondsbw
Copy link

bondsbw commented Feb 25, 2016

I'm not sure what this really helps. Is this

public class Cat
{
    public void Walk();
    public int Paws { get; set; }
}

public adapter Cat: IAnimal
{
    void Walk()
    {
        adaptee.Walk();
    }

    int Paws => adaptee.Legs;
}

better than this which is supported in C# 6?

public class Cat : IAnimal 
{
    int IAnimal.Legs => Paws;

    public int Paws { get; set; } // part of ICat interface
    public void Walk() { ... }
}

Granted, I fudged a bit and assumed IAnimal.Legs is getter-only. (But also to be fair, you never defined adaptee.Walk().)

I could also do something like the following for method adaptation:

void IAnimal.Walk(int speed) => Prowl(speed);
public void Prowl(int speed) { ... }

Maybe this would be a bit better if it were supported:

void IAnimal.Walk => Prowl;

@mharthoorn
Copy link
Author

bondsbw:

I think this line would fail to compile:
if (animal.Paws > 1)
because IAnimal has no member named Paws.

You are correct, I fixed it.

@lachbaer
Copy link
Contributor

@bondsbw The intend, as far as I understand, is to decorate the third party class Cat with that interface. So he cannot directly put Cat : IAnimal, because he has no access to the source code.

But wouldn't this be acceptable, in respect to the code overhead?

public class CatAnimalAdapter : IAnimal 
{
  private CatAnimalAdapter _cat;
  public AnimalAdapter(CatAnimalAdapter value) { this._cat = value; }
  public int IAnimal.Legs => _cat.Paws;
  public void Walk() => _cat.Walk();
}

It's only 2 lines more.

@bondsbw
Copy link

bondsbw commented Feb 29, 2016

@lachbaer Good point, I forgot that was the goal.

#124 is also similar to this idea.

@mharthoorn
Copy link
Author

@lachbaer:

But wouldn't this be acceptable, in respect to the code overhead?

The benefit would not be primarily in terms of saving two lines in defining the adapter. It would be about separation of concerns and the benefit for using the adapter.

Imagine you have a series of objects which you know can all 'walk'. Some of them implement IAnimal and some of them don't. Like the Cat from above.

Compare the use of a regular adapter, where you must test and instantiate the adapter:

public LetItWalk(object item)
{
    if (item is IAnimal)
    {
        ((IAnimal)item).Walk();
    }
    else if (item is Cat)
    {
        IAnimal cat = CatAnimalAdapter(item);
        cat.Walk();
    }
    else ...

When you have adapted Cat to the interface of IAnimal. You now know that all objects implement the interface design time. And thus you can now simply write:

public LetItWalk(IAnimal animal)
{
    animal.Walk();
 }  

@lachbaer
Copy link
Contributor

@mharthoorn
I wonder if this can be done with an 'abstract' interface

public abstract interface IAnimal {
    void Walk();
    int Legs { get; set; }
}

When you use an abstract interface the compiler can check at compile time if the handed class instance implements all of the methods, events and properties of it. You can then pass any class that fits.

In case of your cat you have to define an additional adapter anyway, to make her paws legs. I think it would be better to stick with the current standard and define a derived class with an corresponding copy constructor

public class CatWithLegs : Cat {
  public CatWithLegs(Cat _originalCat) { /* copy */ }
  public int Legs {
    get => base.Paws; // 7881
    set => base.Paws = value;
  }
}

So, the only addition would be the abstract interface
Addendum: it should not be possible to 'derive' from an abstract interface, like with an abstract class definition, just to stay consistent.

@gafter
Copy link
Member

gafter commented Feb 29, 2016

This is quite similar to #7844.

@alrz
Copy link
Member

alrz commented Feb 29, 2016

@lachbaer Since #9297 is closed,

It should not be possible to 'derive' from an abstract interface, like with an abstract class definition, just to stay consistent.

You must derive from an abstract class to be able to instantiate, how is that consistent? And what it has to do with the compiler "checking" the object to see if it implements all the methods? And if it does, what IL should be emitted? Are you suggesting that the compiler should implicitly implement those interfaces for the static type of the instance? This idea has been discussed in #7844 which doesn't add up much IMO. Same questions can be asked about this proposal itself.

@lachbaer
Copy link
Contributor

@alrz

You must derive from an abstract class to be able to instantiate, how is that consistent?

True, I expressed myself wrong. I meant as you cannot instaciate abstract classes it should not be possible to directly use abstract interfaces. But after a moment of thought this doesn't make any sense. An abstract interface should be usable like an ordinary interface.

Are you suggesting that the compiler should implicitly implement those interfaces for the static type of the instance?

No. The compiler just checks if all methods, etc. that are defined in that interface, are also present in the class, though the class does actually not implement the interface. I.e. that you can pass any class to a function that expects this 'abstract' interface.

@HaloFour
Copy link

@lachbaer

No. The compiler just checks if all methods, etc. that are defined in that interface, are also present in the class, though the class does actually not implement the interface. I.e. that you can pass any class to a function that expects this 'abstract' interface.

Exactly what does the compiler do at that point, though? The type still wouldn't implement the interface or have the necessary vtable slots filled to be treated as that interface. The best that the compiler could do is to emit a proxy implementation that delegates the members but you couldn't cast that back to the original type.

@alrz
Copy link
Member

alrz commented Feb 29, 2016

@HaloFour That could work (probably what #7844 suggests?), cast back can be addressed by a conversion operator, but I'm not sure how is that any good.

@HaloFour
Copy link

@alrz

But that only works if the compiler detects the operator and explicitly calls it. To the CLR the types are still incompatible and the isinst and castclass opcodes will fail. This is particularly a problem with generics constrained by this "interface" as the compiler couldn't emit calls to conversion operator methods.

@lachbaer
Copy link
Contributor

But isn't this a general problem of this proposal? After all we're talking about applying a code contract to a class that does not directly implements the interface in question.

@alrz
Copy link
Member

alrz commented Feb 29, 2016

@HaloFour on the other hand, why would you want to cast it, the proxy, back? You want an interface view of it, this cast woudn't be safe generally.

The proxy generation doesn't seem right though, I'd prefer something like #8127 to explicitly state that which implementations are plugged to which classes.

@DavidArno
Copy link

I agree with @grafter that this really is the same thing as proposed for #7844. To work properly, it would require changes to the CLR and would have to be something that is checked/applied at runtime; not at compile-time. If/when the CLR team plan to support structural/duck typing, then a discussion could be had on how to add it to C#. Until then, it's a fairly moot discussion.

Further, as @lachbaer points out, it really doesn't take much extra code to use composition to create a wrapper class, the implements the interface, around a class that doesn't implement it (assuming one keeps their interfaces small of course!)

@mharthoorn
Copy link
Author

@gafter, the main difference between an interface adapter and structural typing is that the latter is a form of duck typing, wich can create loopholes in your type system. Consider the example from #7844 with a different class signature:

structural interface IWriter
{
    void Write(string s);
}   

public class MyWriter
{
    public void WriteLowercase(string s);
    public void WriteUppercase(string s);
}

public void Main()
{
    IWriter w = new MyWriter(); // this is not trivially resolvable.
}

Wich is unproblematic with an explicit mapping.

adapter MyWriter : IWriter
{
    Write(string s)
    {
        adaptee.WriteLowercase(s);
    }
}

@DavidArno
Copy link

@maartenba,

adapter MyWriter : IWriter
{
    Write(string s)
    {
        adaptee.WriteLowercase(s);
    }
}

seems incomplete. You aren't specifying either the type name for the adapter, or what it adapts. Surely it would have to be, eg:

class MyWriterAdapter : IWriter adapts MyWriter
{
    Write(string s) => adaptee.WriteLowercase(s);
}

Which, whilst it's nice syntactic sugar for:

class MyWriterAdapter : IWriter
{
    private MyWriter _adaptee = new MyWriter();
    public void Write(string s) => _adaptee.WriteLowercase(s);
}

it doesn't really save much typing and surely opens up a can of worms of little details around constructors, injecting the _adaptee instance versus creating it within the class etc?

@maartenba
Copy link
Contributor

I think you mean @mharthoorn :)

@mharthoorn
Copy link
Author

@DavidArno, I think you are failing to understand my proposal.
I am not proposing a class. I am proposing an adapter, which can not be instantiated. See my initial text. In other words, we are definitely not talking about syntactic sugar here.

You should read the followoing

public adapter MyWriter : IWriter

as:

adapt MyWriter so that it implements IWriter

@mharthoorn
Copy link
Author

If it helps, the syntax could be:

    public adapt MyWriter : IWriter
    {
        void Write(string s)
        {
            this.WriteLowercase(s);
        }
    }

@DavidArno
Copy link

@maartenba,

I might have done... :)

@HaloFour
Copy link

HaloFour commented Mar 1, 2016

@mharthoorn

How exactly do you see this "adapter" syntax being implemented? For the CLR to be satisfied you will be required to provide an instance of some type that implements the "adapted" interface. You'd need to provide a proxy. Why wouldn't the "adapter" type be that proxy?

@mharthoorn
Copy link
Author

@HaloFour, I can very well imagine that the CLR has problems with it, so @DavidArno might be right that it needs a change to the CLR.

My guess any solution would rather involve extensions (#6136) and / or makes use of the explicit interface mechanism. Because having a proxy probably creates a cascade of new problems.

@mharthoorn
Copy link
Author

Can be a solution for a missing IDisposable implementation too: #10361

@mharthoorn
Copy link
Author

Related: #3357.
(Semantically equivalent)

@iam3yal
Copy link

iam3yal commented Jun 20, 2016

I like the idea but I think that adapter or even adapt is too narrow of a syntax in terms of usage so I'm sold on extension as proposed here #11159. :)

@HaloFour
Copy link

@eyalsk

That proposal specifically mentions that adding interfaces to the base isn't permitted. While I agree it might be neat to be able to extend a class in such a way, without CLR support for it I'm afraid it would be just a weird hack with unexpected limitations. Specifically in that the conversion from concrete class to the interface type would be one-way since it would have to be accomplished through a proxy class.

I think that delegation is probably the better route. It requires the developer to be explicit which in turn should make them quite aware of what limitations may exist. It might be an interesting exercise for code generation:

public class Foo {
    public void Dispose() { }
}

[Delegates(
    Type = typeof(Foo),
    Interface = typeof(IDisposable),
    Implementation = Implement.Explicitly
)]
public partial class FooDisposable { }

/// generated

public partial class FooDisposable : IDisposable {
    private readonly Foo foo;

    public FooDisposable(Foo foo) {
        this.foo = foo;
    }

    void IDisposable.Dispose() { foo.Dispose(); }
}

@iam3yal
Copy link

iam3yal commented Jun 20, 2016

@HaloFour Yeah I know, I read it too but I'd really prefer to have a single feature to extend existing types rather than having multiple extensibility points, we already have one way to do it, do we really want to add a 3rd? personally I'd rather wait and do it right if it make sense.

@Thaina
Copy link

Thaina commented Oct 1, 2016

I have propose another solution as #9595 and #14232

@gafter
Copy link
Member

gafter commented Mar 27, 2017

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https://github.com/dotnet/csharplang/issues, and there is a draft spec at https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https://github.com/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https://github.com/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to #18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.


This particular feature request would be satisfied by type classes (dotnet/csharplang#110) which are under consideration for C#.

@gafter gafter closed this as completed Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests