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: Property templates #10654

Closed
bradphelan opened this issue Apr 18, 2016 · 19 comments
Closed

Proposal: Property templates #10654

bradphelan opened this issue Apr 18, 2016 · 19 comments

Comments

@bradphelan
Copy link

bradphelan commented Apr 18, 2016

Extend the language to allow the creation of generic properties. A generic property is all the logic and storage that a single property on a class may require. It encourages reuse and DRY. It is not a general macro system but encapsulates the property pattern that is already well understood in C#.

One may define the following example class with all the noise we are used to expect.

public class Foo {
    public void PropertyChanged<T>(string name, T value){
        Console.WriteLine($"{name} changed to value {value}");
    }

        public string _Name;
    public string Name {
        get { return _Name;}
        set { _Name = value; PropertyChanged("Name", value);
        }

        public int _Age;
    public int Age {
        get { return _Age;}
        set { _Age = value; PropertyChanged("Age", value);
        }

}

We can DRY this up by the following process.

First define an interface for the type of object you wish to attach your properties to.

public interface IReactive {
    public void PropertyChanged<T>(string name, T value);
}

then at the same scope that classes are defined declare a generic property by introducing a keyword property to distinguish it from classes and namespaces. The property keyword would be parameterized by a type that constrains they type of classes that property can be attached to. The name of the property (in this case ReactiveProp) is also parameterized to specialize the code and storage requirements of the property.

public property<TParent>  ReactiveProp<TProp>
where TParent : IReactive
{
    TProp storage;
    get { return storage; }
    set
    {
        storage = value;
        PropertyChanged(nameof(.), value);
    }
}

The property would need to know it's own name so nameof(.) might be a good way to indicate that. Maybe another method would work better??.

Now create the class you want to attach your properties to. In this case there is the support method PropertyChanged exposed through the IReactive interface that all properties can access.

public class Foo : IReactive {
    public void PropertyChanged<T>(string name, T value){
        Console.WriteLine($"{name} changed to value {value}");
    }

    public property ReactiveProp<string>  Name;
    public property ReactiveProp<int>  Age;
}

Now we can test our new class.

public void Main(){
    var foo = new Foo();

    foo.Name = "Brad";
    foo.Age = 99;
}

Output would be

Name changed to Brad
Age changed to 99

This reduces the line count in the class from 17 lines to 7 lines which is a huge improvement.

This would require support from #850

@Suchiman
Copy link
Contributor

There's also the alternative and more versatile proposal here #5292 and #5561

@bradphelan
Copy link
Author

Those two proposals are essentially macro expanding/code generation
proposals and the dot net team have been pretty explicit in saying no to
that kind of thing. Mainly I think because it is hard to do static analysis
and tooling for visual studio around them. I'm trying to stay narrow.

On Mon, 18 Apr 2016 13:49 Robin Sue, notifications@github.com wrote:

There's also the alternative and more versatile proposal here #5292
#5292 and #5561
#5561


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#10654 (comment)

@axel-habermaier
Copy link
Contributor

axel-habermaier commented Apr 18, 2016

Code generation is currently being considered (#5561, #5292) and would probably be a better way to solve your issue. The way the features are currently planned would allow them to integrate seamlessly with VS.

@DavidArno
Copy link

DavidArno commented Apr 18, 2016

If the suggestions around properties and expression bodies for get/set were implemented (eg see #7881 and #8364), then the code could be reduced to just one more line than your example, without any need to implement interfaces or having any magic code generation going on:

public class Foo 
{
    private T PropertyChanged<T>(string name, T value) 
    {
        Console.WriteLine($"{name} changed to value {value}");
        return value;
    }

    public string Name { get; set => PropertyChanged(nameof(Name), value); }
    public int Age { get; set => PropertyChanged(nameof(Age), value); }
}

@bradphelan
Copy link
Author

@DavidArno INPC is a simple case and an easy straw man to demolish. I hate writing dependency properties. It's a real drag and all the boilerplate obscures the code.

public property<TControl> DependencyProperty<T> {
where TControl : DependencyObject
{
    get { return (Boolean)this.GetValue(nameof(.)); }
    set { this.SetValue(StateProperty, value); } 

    public static readonly DependencyProperty Property

        = DependencyProperty.Register
                       ( nameof(.)
                       , typeof(T)
                       , typeof(TControl)
                       , new PropertyMetadata(default(T));
}

public class Foo : DependencyObject {

    property DependencyProperty<string> A = "Hello";
    property DependencyProperty<int> B = 10;
    property DependencyProperty<double> C = 20;
}

Now to get the above working with current dependency object technology the actual name of Property would have to be scoped correctly. If it got generated as AProperty, BProperty, and CProperty then the dep property name resolution would work.

Note that the above is all statically typed. There is no requirement to decend into T4 template hell. I've written enough code generators to know that it is not fun.

However I agree that the above is probably not general enough. It seems to me that my proposal looks like some kind of lexical scoping that allows composing classes out of smaller bits. The injected class bits can access the outer class via declared interfaces or classes.

@bradphelan
Copy link
Author

Though this link http://www.infoq.com/news/2016/04/CSharp-7 does seem to suggest that code generation will make it in with the "replace" keyword. Maybe I'm wrong. Sounds like macro expansion to me :)

@alrz
Copy link
Member

alrz commented Apr 18, 2016

@bradphelan As others mentioned, #5292 and #5561 are the perfect fit for this scenario,

public partial class Foo : DependencyObject {
  public string Name { get; set; }
}

// to be generated
public partial class Foo {
  public static readonly DependencyProperty NameProperty =
    DependencyProperty.Register("Name", typeof(string), typeof(Foo));

  replace public string Name {
    get { return (string)GetValue(NameProperty); }
    set { SetValue(NameProperty, value); }
  }
}

Also, this is currently planned for C# 7.0 (#2136).

@DavidArno
Copy link

@alrz,

"perfect fit" might be going a little far. If one is prepared to turn all such classes into partial classes and implement a code generator that gets run as part of the build process, then it's a fit. Not sure that makes it "perfect" though, as it seems more like a hack to me at least.

@bradphelan
Copy link
Author

bradphelan commented Apr 18, 2016

Yeah. I'm slowly coming around to it. Thanks for the rewrite of my example. What I don't see here.

public partial class Foo : DependencyObject {
  public string Name { get; set; }
}

is the intention in the hand written code that Name should be replaced and what pattern it should be replaced with. What if there was another property and I wanted a different pattern to be applied. Where do I go to look in the solution to navigate to the thing that generates this? Will the templates be applied by assigning attributes. eg.

public partial class Foo : DependencyObject {
  [PartialDependencyProperty]
  public string Name { get; set; }
}

@axel-habermaier
Copy link
Contributor

axel-habermaier commented Apr 18, 2016

@DavidArno: In my opinion, code generation is way to undervalued. There are lots of very useful things that you can do with code generation, and if Roslyn finally supports a nice way to integrate it into the build process, then great! A lot of feature/language design requests for C# and VB here in github can be tackled with code generation; if the problem and the code generator is generally applicable, there's nothing from stopping the Roslyn team to integrate it into the language proper. So I don't see code generation has a hack, in general. Especially since you don't want to build in (too much) framework-specific stuff into the language, such as dependency properties or INotifyPropertyChanged, for instance.

@alrz
Copy link
Member

alrz commented Apr 18, 2016

@bradphelan That's totally up to you. The generator API provides syntax nodes for you to examine and this is nothing like macro expansions - I'm not 100% sure on the details though. I suggest you to read those proposals and ask if you had questions.

@DavidArno I think writing generators are a one-time thing and I expect they will be available as NuGet packages for you to use. As for making classes partial see #6953.

@khellang
Copy link
Member

@bradphelan You can read more about source generators in generators.md.

The plan is to bring them in the same way Roslyn analyzers are brought in today. They simply take in a SourceGeneratorContext that contains the compilation (among other things). What you do to this is up to you. You can basically inject whatever code you want in there. If you want to look at some sample code (from a super early prototype), see injectors.

As for debugging and viewing generated source, I think the plan is to add a new node in the solution explorer, that would contain the generated source. This would allow you to step through and debug it with VS as well.

@khellang
Copy link
Member

Also, if you want to see source generators demoed, take a look at the "The Future of C#" talk from BUILD. The generators are demoed around 54mins. It's really cool stuff.

@alrz
Copy link
Member

alrz commented Apr 18, 2016

@axel-habermaier Re "A lot of feature/language design requests for C# and VB can be tackled with code generation" Actually records or even ADTs can be simulated with generators until vnext. 😄

@khellang
Copy link
Member

@alrz Which is exactly what one of the early prototypes of injectors did 😝

@axel-habermaier
Copy link
Contributor

@alrz: Yes, indeed. I've recently begun to consider code generation whenever I see a new language feature proposal. It's amazing how much you can do without explicit language support if source generation is nicely integrated into the build process. It's true that real macro functions or code quotations ala F# would be nice, but I'm sure someone is going to build a nice library on top of what C# 7 is hopefully going to provide.

@DavidArno
Copy link

DavidArno commented Apr 18, 2016

I hadn't realised that the plan was to build code generators into Roslyn. In that case, I take back my "hack" comment. This really does open up so many possibilities.

This means we can ship NuGet packages supporting records and discriminated unions (DU's) on the same day as C# 7 ships, rather than having to wait until v8 for those features.

Suddenly, @alrz's request in #10153 for extension operators makes sense as a way of implementing DU's. I could have, eg:

[Union]
partial struct SomeUnion
{
    int IntValue { get; }
    string StringValue { get; }
}

which can be turned into a usable union. However, currently, the is operators for pattern matching would have to be defined in int and string types to achieve eg if (someUnion is int(var value)) ...

@per-samuelsson
Copy link

Just stumbled on this issue, after recently posting #10882. From the tone in here, I feel a little calmer that this source generator thingy actually will surface in a not to distant time.

@gafter
Copy link
Member

gafter commented Mar 24, 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.

I am not moving this particular issue because I don't have confidence that the LDM would likely consider doing this in the form proposed. I would expect that, once (if/when) source generators become a language feature, there would be an add-on using that to handle these use cases.

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