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

Primary constructors #6997

Closed
Richiban opened this issue Nov 24, 2015 · 22 comments
Closed

Primary constructors #6997

Richiban opened this issue Nov 24, 2015 · 22 comments

Comments

@Richiban
Copy link

What ever happened to primary constructors? They were a feature in the C# 6 beta that got dropped (due to time constraints?) but I don't see any mention of them in the discussions for C# 7.

To recap, let's look at the extremely common scenario of a constructor who's purpose is to set private fields, aka the dependencies of the class:

    public class Service
    {
        private readonly ICustomerRepository _customerRepository;

        public Service(ICustomerRepository customerRepository)
        {
            _customerRepository = customerRepository;
        }
    }

The frustration with this code is the duplication and noise: I've had to write the variable name "customerRepository" (or something like it) four times just to say "this class requires an ICustomerRepository". Imagine how cluttered it gets for classes that have two, three, four dependencies. Since this is something that many of us write again and again and again every day, why isn't there language support for this pattern?

Let's look at what F# has for the same scenario:

    type Service(customerRepository : ICustomerRepository) =
        //...

That's it. A simple language feature that allows us to express the very common case of a class that has a dependency on an instance of another type.

The C# syntax would be very similar, as below:

    public class Service(ICustomerRepository _customerRepository)
    {

    }

Any other constructors you write in that class definition must call the primary constructor.

@HaloFour
Copy link

Got wrapped into records I believe. The primary constructor is explicitly mentioned as syntax for them in #206, 1.1.1.

@Richiban
Copy link
Author

Hmm, records is a completely separate issue, unfortunately. It doesn't help in the situations I've described above. Does this mean there are no plans to introduce this feature?

Also, what does #202 reference? GitHub thinks it's a link to issue 202 which is "Formatter ignores NewLine option".

@HaloFour
Copy link

Oops, that's #206. Updating my comment.

@HaloFour
Copy link

As for primary constructors in general, I'm not a huge fan, mostly because of the long and drawn out argument on CodePlex that seemed to want to force them into being both shorthand as well as a fully-capable alternative syntax to current constructors. The additional syntax proposed to bridge the gap between the two was, in my opinion, to put it nicely, "messy and unattractive".

@Richiban
Copy link
Author

Are you referring to the ability to run arbitrary code in the body of the primary constructor? Something like this (IIRC):

public class Service(ICustomerRepository repository)
{
    {
        if(repository == null)
        {
            throw new ArgumentNullException(nameof(repository));
        }
    }

    public Customer GetCustomer(int id)
    {
        //....
    }
}

@HaloFour
Copy link

@Richiban The initializer syntax was one of them (Hello Java). Initializers could also refer to the primary constructor arguments which could led to some bizarre ambiguities, like private readonly ICustomerRepository repository = repository;. I seem to recall there being some scope issue as well but I don't recall what it was.

@alrz
Copy link
Member

alrz commented Nov 24, 2015

To not copy Java syntax, I believe a statement-list would be enough.

public class Service(ICustomerRepository repository)
{    
    if(repository == null) throw new ArgumentNullException(nameof(repository));
    let data = repository.GetData();
    public Customer GetCustomer(int id) { ... }
}

let and var in this context would emit class fields.

And with #6788 you should be able to use await.

@HaloFour
Copy link

If anything I think that would be significantly worse. Then you'd have to pick through what appears to be a method body to find where fields are defined. We already have a fully functional constructor syntax. Like auto-properties, primary constructors should be relegated to the simplest of scenarios. If you need more you can easily opt-into the ever-slightly-more-verbose constructor syntax.

@Richiban
Copy link
Author

@HaloFour "Then you'd have to pick through what appears to be a method body to find where fields are defined". Actually, I'm not sure this would be so bad. You can have the compiler insist that constructor code comes before member declarations. I know I sound like an F# nut but I mention it because they've made it work:

type Service(repository : ICustomerRepository) =
    if repository = null then raise (ArgumentNullException "repository")

    let data = repository.GetData()

    member this.GetCustomer id = //...

@alrz
Copy link
Member

alrz commented Nov 24, 2015

@HaloFour It's not about verboseness of the current constructors, the thing is, that way our class wouldn't be a record anymore, right?

@HaloFour
Copy link

@Richiban Being possible and being appropriate are very different things. C# has a more rigid structure than F# and I think that's a Good Thing™. But really, I just think it's silly to try to promote a complete alternative syntax to constructors. Extending the syntax of primary constructors doesn't make new scenarios possible and isn't appreciably more succinct.

@alrz Perhaps, but how complicated should record classes really be? I assume that whatever syntax candy that primary constructors might apply to types in order to make them records could be applied to a normal class manually. It might depend on how complicated that class would become, and records probably shouldn't be that complicated. If an initializer is necessary, I'd much rather have the Java syntax than to spew code around in a class definition (unless records can't have other defined members). But I'd also rather not have the wacky ambiguity and scoping issues that also come with this package.

Look at all of the syntax proposals that arose from primary constructors on CodePlex:

Better support for attributes in captured primary constructor arguments
Alternative “primary constructor” idea
Constructor Shorthand
Validation for primary constructors
Primary constructors syntax
Init autoproperties - a new take on readonly and primary constructors

And of course the various drawn-out arguments within the different issues and meeting notes. All of this is a good thing but much of it feels like people trying to declare out as much of their class on one line as possible, as if newlines slow down their programs.

@alrz
Copy link
Member

alrz commented Nov 24, 2015

@HaloFour Just to say, some people believe that egyptian brackets also make the code run faster. 😄

@dsaf
Copy link

dsaf commented Nov 24, 2015

@alrz
Copy link
Member

alrz commented Nov 24, 2015

@HaloFour How about class delegation

class Foo(private ICustomerRepository repository) : ICustomerRepository using repository
{
}

as an alternative to #6081

@HaloFour
Copy link

@alrz I'm a big fan of Stroustrup-variant K&R, but I wouldn't advocate syntactic changes specific to supporting (or encouraging) it.

@alrz
Copy link
Member

alrz commented Nov 24, 2015

@HaloFour irrelevant to class delegation. :/

@HaloFour
Copy link

@alrz That it is. I didn't comment on that because it's already closed in favor of proper mixins, so it doesn't really matter what I think of the syntax.

@alrz
Copy link
Member

alrz commented Nov 24, 2015

@HaloFour I have no idea how mixins will be look like. Hope they wouldn't be a new kind of types.

@Joe4evr
Copy link

Joe4evr commented Nov 25, 2015

some people believe that egyptian brackets also make the code run faster.

Or compile faster.

@gafter gafter added the Resolution-Answered The question has been answered label Dec 1, 2015
@ymassad
Copy link

ymassad commented Dec 12, 2016

I think that this feature can be created gradually. First, the simplest (as described in the first post of this thread) support for primary constructors can be added to the language. And then there would be a lot of time to discuss if/what more features should primary constructors support.

@gafter
Copy link
Member

gafter commented Dec 12, 2016

@ymassad If we were to use the record syntax to introduce primary constructors, it would be an incompatible change to later add the things that would make them records (without additional, undesireable syntax). So that approach is unlikely to work out well.

@ymassad
Copy link

ymassad commented Dec 13, 2016

@gafter, I have taken a look at the record type syntax. If they allow record types to have a class body without a special syntax (e.g. public record class...), it would mean that record types are distinguishable from normal classes only by having a primary constructor, not by some other special syntax. That would be a big mistake in my opinion.

I suggest that record types either have no class body, or that they should have a record modifier if they have a class body.

This way, non-record classes can have primary constructors.

@gafter gafter closed this as completed Jan 5, 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

7 participants