Replies: 36 comments 2 replies
-
Something similar was prototyped for C# 6 (Primary Constructor) before the team decided to delay that and rework it into Records (#39). |
Beta Was this translation helpful? Give feedback.
-
@Joe4evr And it's still a massive disappointment to me that they did. After all, primary constructors have nothing to do with records (other than the fact that records automatically get a primary constructor). The only reason they did this as far as I can tell is because they wanted to steal the syntax I've copied a bit of code from a few classes within reach of me right now that show why I want such a feature so badly: public sealed class CompleteCheckoutCommandHandler : ICompleteCheckoutCommandHandler
{
private readonly ICreateNewOrderForCheckoutCommandHandler _createNewOrderForCheckoutCommandHandler;
private readonly ICommandSender _commandSender;
private readonly ITakePaymentsCommandHandler _takePaymentsForOrderCommandHandler;
private readonly IGetCustomerPaymentSourceQueryHandler _getCustomerPaymentSourceQueryHandler;
public CompleteCheckoutCommandHandler(
ICreateNewOrderForCheckoutCommandHandler createNewOrderForCheckoutCommandHandler,
ICommandSender commandSender,
ITakePaymentsCommandHandler takePaymentsForOrderCommandHandler,
IGetCustomerPaymentSourceQueryHandler getCustomerPaymentSourceQueryHandler)
{
_createNewOrderForCheckoutCommandHandler = createNewOrderForCheckoutCommandHandler;
_commandSender = commandSender;
_takePaymentsForOrderCommandHandler = takePaymentsForOrderCommandHandler;
_getCustomerPaymentSourceQueryHandler = getCustomerPaymentSourceQueryHandler;
}
// Methods removed
} internal sealed class DocumentDbBasketRepository : IBasketRepository
{
private readonly IDocumentDbBasketMapper _domainToDataDocumentDbBasketMapper;
private readonly DocumentClient _basketClient;
private readonly DocumentDBContextInfo _documentDbContextInfo;
private readonly IBasketOwnerMapper _basketOwnerMapper;
private readonly IBasketItemMapper _basketItemMapperToDomain;
private readonly IGetProductImageUrlsQueryHandler _getProductImageUrlsQueryHandler;
internal DocumentDbBasketRepository(
DocumentClient basketClient,
DocumentDBContextInfo documentDbContextInfo,
IDocumentDbBasketMapper domainToDataDocumentDbBasketMapper,
IBasketOwnerMapper basketOwnerMapper,
IBasketItemMapper basketItemMapperToDomain,
IGetProductImageUrlsQueryHandler getProductImageUrlsQueryHandler)
{
_basketClient = basketClient;
_domainToDataDocumentDbBasketMapper = domainToDataDocumentDbBasketMapper;
_basketOwnerMapper = basketOwnerMapper;
_basketItemMapperToDomain = basketItemMapperToDomain;
_getProductImageUrlsQueryHandler = getProductImageUrlsQueryHandler;
_documentDbContextInfo = documentDbContextInfo;
}
// Methods removed
} public class PercentageValueDiscountService : IPercentageValueDiscountService
{
private readonly IDiscountRepository _discountRepository;
public PercentageValueDiscountService(IDiscountRepository discountRepository)
{
_discountRepository = discountRepository;
}
// Methods removed
} Now, in our entire codebase the vast majority of our classes look like this. It's incredibly frustrating to have to write these stupid constructors over and over again. Are we alone in this? Are we the only ones writing code where 90% of all constructors do nothing but set (private readonly) fields? To my eye they are overly verbose/repetitive, take up far too much space (obscuring the actual logic of your class which has only a single public method) and they're a hiding place for bugs. I'd love to use an IL-weaving project such as Fody to just auto generate them all but I've tried it and it completely screws the design-time experience (ReSharper just doesn't understand what's going on at all and you get an awful lot of good-code-red). Why did the team abandon the idea of primary constructors? Why can't I write (in the last example): public class PercentageValueDiscountService(IDiscountRepository discountRepository) : IPercentageValueDiscountService
{
// Methods removed
} NB I know that records might enable a slightly shorter syntax for this, but these types are not records. They should have neither equality semantics nor public properties exposing these dependencies. PS Sorry for the rant. I just really want this. |
Beta Was this translation helpful? Give feedback.
-
Copying from my comment on #39: For reference, here are other languages which all support both primary constructors and records: F#: type Greeter(name : string) =
member this.SayHello () = printfn "Hello, %s" name Scala: class Greeter(name: String) {
def SayHi() = println("Hello, " + name)
} Kotlin: class Greeter(val name: String) {
fun greet() {
println("Hello, ${name}");
}
} |
Beta Was this translation helpful? Give feedback.
-
IIRC it was probably because the original conversation went down a path that was trying to turn primary constructors into an alternative syntax for full-fledged constructors complete with bizarre initialization and scoping syntax/rules. By tying the feature to records it limits the use cases. Why not use field injection? It would eliminate the need for those constructors. Or why not use quick-refactorings to write the constructors for you? It's literally That said I don't see a real reason for primary constructors to be necessarily limited to records, but I would hate to see it go down the same path it did before. A succinct form of the syntax for limited but common scenarios would be good enough, and an additional IIRC, all of your primary constructor examples in other languages result in publicly exposed fields, which is probably not what you want. |
Beta Was this translation helpful? Give feedback.
-
Unfortunately that would significantly weaken the design; it would now be possible to construct these service types with null dependencies. As far as I'm aware no one recommends field injection any more and some IOC frameworks have even stopped supporting it.
I am doing that currently. Writing the constructor isn't difficult, but reading and maintaining it are both an unreasonable burden.
I absolutely agree. I'm pretty sure that a syntax that simply declares and sets fields would already cover 95% of cases. It doesn't need to cover 100%: anyone with more custom needs is able to use the existing syntax.
Edit: verified all three examples No, they're private: type Greeter(name : string) =
member this.SayHello () = printfn "Hello, %s" name
let g = Greeter("Bob")
g.name // Error: The field, constructor or member 'name' is not defined Scala: class Greeter(name: String) {
def SayHi() = println("Hello, " + name)
}
val g = new Greeter("Bob");
System.out.println(g.SayHi());
System.out.println(g.name); // Error: value name is not a member of Playground.this.Greeter Kotlin: class Greeter(val name: String) {
fun greet() {
println("Hello, ${name}");
}
}
fun main(args: Array<String>) {
val g = Greeter("Joe");
System.out.println(g.greet())
g.firstName // Error: Unresolved reference: firstName
} |
Beta Was this translation helpful? Give feedback.
-
Field injection? Yuck. |
Beta Was this translation helpful? Give feedback.
-
Apologies, I made a mistake with the Kotlin example (I actually typed the wrong field name). @HaloFour was right and it does need a class Greeter(private val name: String) {
fun greet() {
println("Hello, ${name}");
}
}
fun main(args: Array<String>) {
val g = Greeter("Joe");
System.out.println(g.greet())
g.name // Error: Cannot access 'name': it is private in 'Greeter'
} |
Beta Was this translation helpful? Give feedback.
-
Maybe: public class Dog
{
public [in] string Name;
}
...
var dog = new Dog("Hoffer");
dog.Name; // Hoffer Although it is necessary to consider how to set the order... public class Dog
{
public [in] string Name;
private [in] short Age;
// or
public [in] short Age { get; [private] or [protected] set; }
}
...
var dog = new Dog(Name: "Hoffer", Age: 4);
dog.Name; // Hoffer
dog.Age; // AccessException What do you think? |
Beta Was this translation helpful? Give feedback.
-
@Richiban I don't like your example because you get inheritance list just being moved to another screen for large enough constructors. Consider your own example: internal sealed class DocumentDbBasketRepository : IBasketRepository
{
private readonly IDocumentDbBasketMapper _domainToDataDocumentDbBasketMapper;
private readonly DocumentClient _basketClient;
private readonly DocumentDBContextInfo _documentDbContextInfo;
private readonly IBasketOwnerMapper _basketOwnerMapper;
private readonly IBasketItemMapper _basketItemMapperToDomain;
private readonly IGetProductImageUrlsQueryHandler _getProductImageUrlsQueryHandler;
internal DocumentDbBasketRepository(
DocumentClient basketClient,
DocumentDBContextInfo documentDbContextInfo,
IDocumentDbBasketMapper domainToDataDocumentDbBasketMapper,
IBasketOwnerMapper basketOwnerMapper,
IBasketItemMapper basketItemMapperToDomain,
IGetProductImageUrlsQueryHandler getProductImageUrlsQueryHandler)
{
_basketClient = basketClient;
_domainToDataDocumentDbBasketMapper = domainToDataDocumentDbBasketMapper;
_basketOwnerMapper = basketOwnerMapper;
_basketItemMapperToDomain = basketItemMapperToDomain;
_getProductImageUrlsQueryHandler = getProductImageUrlsQueryHandler;
_documentDbContextInfo = documentDbContextInfo;
}
// Methods removed
} whould become internal sealed class DocumentDbBasketRepository(
DocumentClient basketClient,
DocumentDBContextInfo documentDbContextInfo,
IDocumentDbBasketMapper domainToDataDocumentDbBasketMapper,
IBasketOwnerMapper basketOwnerMapper,
IBasketItemMapper basketItemMapperToDomain,
IGetProductImageUrlsQueryHandler getProductImageUrlsQueryHandler) : IBasketRepository
{
// Methods removed
} And when you have even more parameters (for complex enough type) it hides all base classes/interfaces. I'd prefer something like: internal sealed class DocumentDbBasketRepository : IBasketRepository
with constructor(
DocumentClient basketClient,
DocumentDBContextInfo documentDbContextInfo,
IDocumentDbBasketMapper domainToDataDocumentDbBasketMapper,
IBasketOwnerMapper basketOwnerMapper,
IBasketItemMapper basketItemMapperToDomain,
IGetProductImageUrlsQueryHandler getProductImageUrlsQueryHandler)
{
} But I see here an issue with field naming. For example, should these fields have |
Beta Was this translation helpful? Give feedback.
-
@Pzixel I don't think that the field naming convention is much of an issue; the underscore prefix is only a naming convention and it's far from universal. I foresee that most teams will switch naming convention if they have to for either the constructor parameters (little effect on code outside the class) or the fields (no effect outside the class). As for subtypes being pushed down, again I'm fine with that. I don't think I've ever seen a class with a list of dependencies long enough to literally push the end of the parameters list off the screen (my standard 1080p screen can display about 45 lines). It might be reasonable to argue that with 40+ dependencies in a class you've got other problems 😝 I think it's obvious enough that the first line: internal sealed class DocumentDbBasketRepository( is unfinished and reader's eye must continue before they should expect the list of subtypes. |
Beta Was this translation helpful? Give feedback.
-
Beta Was this translation helpful? Give feedback.
-
@0xF6 I really like the syntax I think it's much more readable then putting anything in constructor definition. And it could simply translate from
to
And inheritance could work with few caveats , if you had
It would be the same as
Some naming rules should be defined like if there are two params with same name and optional params but this would make sense to me. You would get everything you get with constructor injection but with the nice syntax of property injection. |
Beta Was this translation helpful? Give feedback.
-
It could be problematic because by adding or annotating a field you'd be creating breaking changes in the constructors without seeing it. |
Beta Was this translation helpful? Give feedback.
-
I don't see how that would work with |
Beta Was this translation helpful? Give feedback.
-
@HaloFour Is this really such a big issue? I see this as a way to simplify a particular use case for DI. So disabling this for a partial class would not be that big of a deal. Or allowing only one part to use in fields in the class. It could also make two different constructors one for each part and let you deal with combining the two. |
Beta Was this translation helpful? Give feedback.
-
@Pzixel You are right about the constructor, but what I noticed is that I generally stop using immutable structures or not use object initializers. And I like both, since you can only do this during construction private could theoretically work, BUT I'm not saying this has to work with private I would be happy 😊 if it only worked with public properties. The private part can be dropped if people don't think it's useful i just think this makes more sense, but I don't think this would work without readonly properties (Prop4 in the example). This is just an idea to get people to think about it. |
Beta Was this translation helpful? Give feedback.
-
But if it's applicable to public properties only it becomes almost useless. You can see an example above, in most cases I ever seen you init private fields of class because otherwise you can get this value changed. For example if you get list of something as parameter you don't want expose it because someone may modify it. Make all properties completely immutable? It may be not applicable always and it also makes your codebase 2x larger because there is built in |
Beta Was this translation helpful? Give feedback.
-
When I write mine that way, the |
Beta Was this translation helpful? Give feedback.
-
@jnm2 hmm, why then my |
Beta Was this translation helpful? Give feedback.
-
It's not my fault they're dumb... 😉 |
Beta Was this translation helpful? Give feedback.
-
Not necessarily this will become useful with Non-Nullable types. It will be simpler to force someone to put a value and not use default. I am still not sure how this will work when they implement non-nullable.
Making the field readonly makes sure the value won't change, it can be public or private. The "immutable" property is the Prop4 in the example. But in regards to private properties, to me |
Beta Was this translation helpful? Give feedback.
-
Making |
Beta Was this translation helpful? Give feedback.
-
When I initially created this issue I actually thought it would turn out simpler than this 😊 @ahejlsberg do you have any thoughts on this? |
Beta Was this translation helpful? Give feedback.
-
I still think Richiban had the best solution.
Concise, obvious, no likely confusion with other syntax, the constructor is still a constructor doing constructor things, no new keywords required, minimal new syntax with no drastic re-interpretations around it, and the behind-the-scenes part is exactly like automatic backing variables for properties, which was acceptable to just about everyone. I see no downside. |
Beta Was this translation helpful? Give feedback.
-
I hadn't thought of Typescript! Good idea; here's an example: class Greeter
{
constructor(private name: string) { }
sayHello() { return `Hello, ${this.name}`; }
} Looks pretty nice, IMO. It also solves the problem of interface implementation declarations being pushed down the page that @Pzixel mentioned (#1087 (comment)). |
Beta Was this translation helpful? Give feedback.
-
I like the TypeScript approach. However, there are some problems with lifting that syntax directly into C#. One major difference is that TypeScript permits only one constructor per class; not so with C#. How would you propose a syntax like that working in the presence of multiple constructors, each of which may want to assign a value to that field? Things get even trickier if an "extension everything" proposal includes extension constructors. I don't believe anyone has an appetite for extension members adding state to a class. |
Beta Was this translation helpful? Give feedback.
-
That's why it might be an idea to have these primary constructors look a bit different to ordinary ones. If we take a leaf out of F#''s book, once fields are defined the constructor becomes a primary constructor and all other constructors must (ultimately) call the primary one. |
Beta Was this translation helpful? Give feedback.
-
We need for the next release C# 8, syntactic sugar that will be a mix between: |
Beta Was this translation helpful? Give feedback.
-
I know this issue is old, but it's still relevant. I'm just looking for an implied constructor (like how a default parameter-less constructor is implied) for derived classes.
|
Beta Was this translation helpful? Give feedback.
-
Let's put it in different way: what you can't write with source generators? I'm pretty sure that with generators of sufficient complexity you can implement any feature 😃 |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
It would be great if we could have some syntactic sugar similar to Typescript to specify class level fields for arguments passed into a constructor. For example, this:
would become this:
Is this possible?
Beta Was this translation helpful? Give feedback.
All reactions