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] Private inheritance #5270

Closed
angularsen opened this issue Sep 16, 2015 · 15 comments
Closed

[Proposal] Private inheritance #5270

angularsen opened this issue Sep 16, 2015 · 15 comments

Comments

@angularsen
Copy link

Problem

It is tedious to wrap a 3rd party type with the intent to expose only a subset of its public members, by implementing and forwarding property/method calls.

Motivation:

  • Simplify the API and make the intent of how to use the type more obvious
  • Prevent bugs by hiding members that can only be called in certain contexts, such as lazy relational properties in an ORM session
  • Easier to wrap types and the intent of the wrapper type is more clear (hide members)

Proposal

Introduce a new inheritance syntax for private inheritance, which allows you to choose what members to make public, either by opt-in or opt-out, whichever is the most convenient. Similar to private inheritance in c++.

Example

Update 2015-09-27: See updated usecases in comments.

Consider the syntax purely for illustration purposes.

Private interface inheritance

// Alternative 1: Opt in / expose certain members of IPerson
public interface IPersonInfo : private IPerson
{
    // Public scope, expose these members
    public IPerson
    {
        string Name { get; }
        string Email { get; }
    }
}

// Alternative 2: Opt out / hide certain members
public interface IPersonInfo : IPerson
{   
   // Private scope, hide these members
   private IPerson
   {
        ICollection<IPerson> Friends { get; }
   }
}

public interface IPerson
{
    string Name { get; }
    string Email { get; }

    // Throws LazyException if accessed outside ORM session scope
    ICollection<IPerson> Friends { get; }

    // Many other properties here...
}

public class Person : IPerson 
{
    // Implementation here
}

Concerns

  • How to distinguish opt in/out when extending multiple interfaces with overlapping members?
  • All the Person object's members are available via casting or reflection, so this should not be used to secure the hidden members
  • Cannot allow both opt in and opt out
  • Tooling and intellisense support
  • How does these cases differ?
    • A class privately inherits another class type
    • A class privately inherits one or more interfaces
    • An interface privately inherits one or more interfaces

I am probably way off here and have likely missed a fundamental flaw, but I found the idea intriguing and could not find any previous discussions on the topic. Would love some feedback and thoughts from people with more insight.

Update 2015-09-18: Remove misleading comment in alternative 2.

@HaloFour
Copy link

What would happen to the members that the type doesn't "opt-in" to? What if you upcast to the parent type or interface and attempt to call those members?

The type is a contract (particularly in the case of interfaces) and I would argue that you shouldn't be able to not implement the required members. Even if you could hide those members they should still be required to exist.

Wouldn't your use case be better served through a proxy?

@HaloFour
Copy link

As for "hiding" members, that is already supported through explicit interface implementation in C#. The CLR also supports a form of "explicit" overriding of virtual members where the name and accessibility of the member can be different, but C# doesn't expose this functionality. In both cases the type can be upcast and those members visible.

@vladd
Copy link

vladd commented Sep 16, 2015

Is there a good use case for this? I think composition (with a kind of yet unimplemented traits = "let inner object implement outer interface") and extension methods cover most (all?) of the use cases.

@orthoxerox
Copy link
Contributor

Your proposal looks like syntactic sugar for composition, not inheritance, am I right? You want to reuse the implementation, but not the interface, plus you don't need subtyping.

@AdamSpeight2008
Copy link
Contributor

Would it also work for sealed types and structures?

@svick
Copy link
Contributor

svick commented Sep 16, 2015

I think this would be of pretty limited use. Wrapping is common, but when you do that, you usually expose the functionality in a different way, you don't just hide some members.

Because of that, I don't think this is worth having special syntax.

@default0
Copy link

I often find myself writing code like this:

public class Books : IEnumerable<Book>
{
    private List<Book> books;

    public Books() { books = new List<Book>(); }

    public void Add(Book book)
    {
        books.Add(book);
    }
    public void Remove(Book book)
    {
        books.Remove(book);
    }
    public IEnumerator<Book> GetEnumerator()
    {
        return books.GetEnumerator();
    }
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return books.GetEnumerator();
    }
}
public class Book
{
    public string Author { get; set; }
    public string Contents { get; set; }
}

Notice a pattern with the methods? Looks pretty wet.
The specific case is: I want to reuse functionality, but I want to define my own interface over it.
Expression-bodied methods help to some extent, but as far as I know, they could only be used for the last two methods, because they require that the method return a value (I do not have VS2015 handy to test this, so I'm not 100% certain). However, in essence all that would be required to make this a lot more succinct and nice would be a rewrite like so:

public class Books : IEnumerable<Book>
{
    private List<Book> books = new List<Book>();

    public void Add(Book book) => books.Add(book);
    public void Remove(Book book) => books.Remove(book);
    public IEnumerator<Book> GetEnumerator() => books.GetEnumerator();
    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => books.GetEnumerator();
}
public class Book
{
    public string Author { get; set; }
    public string Contents { get; set; }
}

Still, this doesn't scale particularly well if I have large interfaces and only want to exclude parts of them (repeating 20+ interface methods, even if I need one line for each, is still pretty wet). However at least for me this gets "close enough" so I wouldn't really be bothered by it.
Another issue is that exceptions thrown by the base implementation (especially those thrown due to invalid arguments) would have a stack-trace pointing down to the List implementation, instead of acting like the methods were truly the same, which is sometimes off-putting (I usually think that some library doesn't do proper validation when I see exceptions in bcl calls made from the library).

tl;dr I think that reducing boiler-plate required for composition may be easier and more worthwhile than introducing a completely new concept to tackle this problem.

@angularsen
Copy link
Author

I think composition (with a kind of yet unimplemented traits = "let inner object implement outer interface") and extension methods cover most (all?) of the use cases. - @vladd link to comment

I agree, I think traits would go a long way. Basically being able to send in Person to a method taking IPersonInfo interface, as long as Person is compatible by implementing all the members of IPersonInfo.

The only thing I don't see traits provide, is improving composition. @default0 points out an alternative syntax, that maybe together with traits would be a good combination?

@AdamSpeight2008
Copy link
Contributor

@default0 You could just inherit from List<Book>

@angularsen
Copy link
Author

@AdamSpeight2008: If I understand @default0 's case correctly, he wants to reuse the list functionality by composition, but only expose a subset of List<> or IList<> members. At least for me, it comes down to a lot of code for simple wrapper types.

Usecases

I'm sure there are many usecases for this, but the ones that I've experienced myself or can think of are.

Hide members to avoid bugs

Ex: PersonInfo hides the lazy relational properties of the ORM type Person when passed out of a DB context scope, as they would throw an exception when accessed.

Hide members to limit the exposed functionality or make the intention more clear

Ex: ReadOnlyCollection that according to source code also just wraps a passed in IList object.

Adapters

Adapter type to connect legacy or third party code to new types.

Private inheritance revisited

After feedback in this thread, I am now leaning towards primarily traits and possibly an improved composition syntax to meet the needs of this issue.

Proposal: Interface private inheritance (use traits instead)

In my original example I wanted the subset type IPersonInfo to be a private extension of IPerson in such a way that a class Person : IPerson object would be accepted in a method that takes IPersonInfo. After feedback in this thread, I think this is best solved by traits. A Person object could then be passed into any method taking any compatible type.

Proposal: Class private inheritance (traits + improved composition syntax)

Similar to the interface example, I envisioned classes being able to privately inherit other classes, similar to C++, and either opt-in or opt-out of making those members public. All the base members would be accessible from within the type. Obviously this might break any interfaces implemented by the base type, so I think we are once more entering traits territory.

Proposal: Improved composition syntax

As pointed out, there may be a benefit of making composition simpler by adding a new syntax for forwarding properties and methods to the base type. I've extended a bit on that example here:

class MyBooks<T>
{
    private IList<T> books;

    // Alt1: Lambda statement, not much benefit over a standard method
    public void Add(Book book) => { books.Add(book) }

    // Alt2: Lambda expression
    public void Add(Book book) => books.Add(book);

    // Alt3: Lambda expression, when identical member signature
    public void Add(Book book) => books;

    // Alt1: Lambda expression for property
    public int Count => books.Count;

    // Alt2: Lambda expression for property
    public int Count => books;
}

Combine this with traits, and I can now pass Books<T> to a method that only cares about calling .Add() or .Count on it. I think this is a big win.

@vladd
Copy link

vladd commented Sep 27, 2015

@anjdreas Jon Skeet's proposal on mixins is here (didn't find it on his blog).

@benaadams
Copy link
Member

I'd like : private IPerson where the private implementer can internally cast to that interface; but external users can't.

e.g. for Stream in https://github.com/dotnet/coreclr/issues/2179

public abstract class Stream : IDisposable, private  IReadStreamAsync<byte>....`
{
    public IReadStreamAsync<byte> AsReadStreamAsync()
    {
        if (!CanRead) __Error.ReadNotSupported();
        return this;
    }
}

So you can do

Stream readonlyStream;
IReadStreamAsync<byte> reader = readonlyStream.AsReadStreamAsync();

But would be prevented at compile time from doing

Stream writeonlyStream;
IReadStreamAsync<byte> reader = (IReadStreamAsync<byte>)writeonlyStream;

@svick
Copy link
Contributor

svick commented Nov 28, 2015

@benaadams How is that better? As far as I can tell, you're just exchanging one runtime exception (InvalidCastException) for another (whatever __Error.ReadNotSupported() throws).

@benaadams
Copy link
Member

@svick its better as its maintaining the contracts.

You'd get a compile error if you tried to cast; so wouldn't be able to; and yes, unfortunately a runtime error for AsReadStreamAsync(). However, you'd also know if AsReadStreamAsync() didn't throw you'd have a readable stream.

The otherway; either Stream can't implement the interface and needs a shim object to be returned by the function; when you'd still get the same runtime error; or an extra allocated object and indirection for the interface shim.

Or you would implement the interface which would allow the cast; but then the user of the interface wouldn't be sure it was a readable stream and calls to Read could throw as it wasn't a readable stream.

Which I think would be worse as its clearly violating the interfaces contract; would happen much further along the call stack; and be harder to work out why and where the non-readable readable stream came from.

@gafter
Copy link
Member

gafter commented Mar 20, 2017

We are now taking language feature discussion on https://github.com/dotnet/csharplang for C# specific issues, https://github.com/dotnet/vblang for VB-specific features, and https://github.com/dotnet/csharplang for features that affect both languages.

@gafter gafter closed this as completed Mar 20, 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

9 participants