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: Type constraint for constructor with parameter #2206

Closed
Whiteknight opened this issue Apr 23, 2015 · 11 comments
Closed

Proposal: Type constraint for constructor with parameter #2206

Whiteknight opened this issue Apr 23, 2015 · 11 comments

Comments

@Whiteknight
Copy link

Current type constraints allow us to specify that the class must have a default constructor:

void Method<T>() where T : new() { ... }

but we cannot specify that a class must provide a non-default constructor. When you consider the generally-recognized benefits of dependency inversion and when you consider that injecting dependencies via constructor is generally considered superior to injecting via properties and initialization methods, this seems like a very good feature to have.

Any time we have to create objects from a family with common dependencies, this could help reduce required code. Consider the example of a factory for creating UI controls. We might take a string with the name of the type you're creating and a UI Context object which is always passed to all constructors:

class ControlFactory {
    ControlBase Create(string type, IUiContext ctx) {
          switch (type) {
              case "button": return new Button(ctx);
              case "scrollbar": return new ScrollBar(ctx);
              ...
          }
      }
}

We could dramatically shrink this method, in addition to improving type safety and removing casts, by converting to generic with a constraint that allowed us to specify a non-default constructor:

class ControlFactory {
    T Create<T>(IUiContext ctx) where T : new(IUiContext) {
        return new T(ctx);
    }
}

Lots of really ugly code involving Activator and reflection over ContructorInfo lists could be removed entirely, in addition to helping support proper dependency inversion of types created generically.

@VSadov
Copy link
Member

VSadov commented Apr 23, 2015

I think I have seen this request more than once already. Currently this runs into a limitation where CLR provides no way to express this.

Another thing to note - "new T" actually uses Activator in its implementation, and Activator internally uses reflection over ConstructorInfo, so in that sense "new T" is not the way to avoid these APIs, it just makes code look nicer.

@ocbaker
Copy link

ocbaker commented May 5, 2015

I would deeply like this feature too. In regards to expression with the CLR you guys said at build that although you don't want to modify the CLR it was not out of reach so to speak. So if you do end up planning on updating the CLR in C# 7 (VB 15?) would this be an unreasonable request?

If new T just uses Activator why do people notice a perf difference between the two implementations?
http://stackoverflow.com/q/6069661/1105080 (It's an old question but has answers for 4.0) (Unless I misunderstood and you are just refering to at design time)

And if new T just uses Activator then wouldn't it be possible for compiling into the current CLR to just use the formatting as syntactic sugar and where the constructor is used convert into the use of Activator?

@gafter
Copy link
Member

gafter commented May 5, 2015

@ocbaker Unfortunately there is no way to use Activator to invoke a selected constructor other than the default constructor. While there is https://msdn.microsoft.com/en-us/library/4b0ww1we(v=vs.110).aspx a version that does some "overload resolution" at runtime, that doesn't work so well in many cases, for example when the actual arguments are null, or when the result of overload resolution at runtime based on the types of the runtime objects would be a different constructor than the one the compiler selected based on the static types.

@svick
Copy link
Contributor

svick commented May 5, 2015

@gafter What about either improving Activator or using a custom code with similar functionality? I think a design that would solve the overload resolution issue would be to encode the types using a delegate type parameter, e.g. (using @Whiteknight's example):

Button button = Activator.GetInstanceCreator<Func<IUiContext, Button>>().Invoke(ctx);

My quick attempt at implementing it using Expression.Compile() or DynamicMethod show much higher time for first call, but also significantly lower times for subsequent calls, especially for the non-default ctor case (where I believe Activator doesn't use caching):

Activator Expression DynamicMethod
default ctor, first call 0.093 ms 7.5 ms (80× worse) 3.8 ms (40× worse)
default ctor, subsequent calls 140 ns 36 ns (3.9× better) 23 ns (6.1× better)
other ctor, first call 0.23 ms 7.7 ms (33× worse) 4.0 ms (17× worse)
other ctor, subsequent calls 1700 ns 37 ns (46× better) 25 ns (68× better)

(Though Activator also does a lot of security checks, my code doesn't do any.)

I believe at least Expression.Compile() should be available on all platforms, so this might be a viable approach.

@default0
Copy link

I just finished the first draft of this library to work around issues like this and similar.
While it doesn't allow using constructors directly, it allows you to restrict types to implement specific static methods, so you can use it to ensure that type T has a static factory method taking a certain number of parameters.
The NuGet Package comes with a Roslyn Analyzer to help you correctly implement static interfaces etc.

It generates IL and types on the fly to achieve this and allows having some level of typesafety as well.
Would that lessen the need for this feature for you?

@IanKemp
Copy link
Contributor

IanKemp commented Jan 13, 2017

Please, please, please can we get this into the language?

@Thaina
Copy link

Thaina commented Jan 26, 2017

Nearly 2 years and still planning....

Anyone could enlighten me why this very crucial feature is so hard to implement?

@gafter
Copy link
Member

gafter commented Jan 26, 2017

@Thaina it requires a revision of the CLI specification, and work on every implementation of the CLR.

@taori
Copy link

taori commented Mar 24, 2017

@gafter Would you recon this feature might make it into c# 8? On some issue i've read that features which require CLR work should be done in combination, to keep required changes to CLR at a minimum

@jnm2
Copy link
Contributor

jnm2 commented Aug 18, 2017

Since we are closing language discussions on Roslyn in favor of csharplang, perhaps we should just close and link to the existing dotnet/csharplang#769?

@gafter
Copy link
Member

gafter commented Oct 2, 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 is part of a proposal being tracked at dotnet/csharplang#110 which is under active development at MSR (Microsoft Research).

@gafter gafter closed this as completed Oct 2, 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

10 participants