-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: Class-scoped blocks for fields / explicit field usage #12361
Comments
When blocks are used in other areas of the language, they define the scope of contained members or locals. Whatever is inside is not accessible to the outside (unless prefixed with an appropriate access modifier, where applicable). This suggestion breaks that pattern. One possible solution is to add a new access modifier such as |
Yes and no. At some points you always encounter some things that break existing rules, because otherwise these things would be much more difficult to realize, i.e. write- and readable. In my opinion this (optically minor) extension would justify this (again - minor -) deviation. And as stated, that block could be prepended by a (preferably existing) keyword to emphasize that in this block only the (implicitly)
Local to what? To a surrounding block? There we are again... |
Yes, local to the surrounding block. Its scope would not leak out of the block, whereas any member which is at least |
@bondsbw Indeed a nice idea and approach. Everything within a class-nested block would be block-scoped only, including fields, methods, properties and sub-classes, unless a modifier is explicitly prepended. Then there is even no need for a public class Person
{
{
int _age; // no modifier -> scoped to this block only
private _name; // private modifier -> class-scope
int _HelperMethod() { /**/ } // no modifier -> scoped to this block only
private int Gender { get; set; } // get and set imply private modifier, so class scoped
}
} |
Agreed that |
I think that would be very confusing to visually parse. Is it really that necessary to limit visibility of members within a class to these additional scopes? If a class is so big that proper maintenance requires such rigid protection from yourself (or team mates) it seems to me that this class is too big. Local variables and functions would also likely suffice in many of those scenarios. |
@HaloFour In a perfect coding world you would also be right regarding the size of a class. But IRL I have already seen so many long classes, just look at Roslyn. Even there are classes that make use of direct access to private fields instead of encapsulating every private field in a property, what is okay. As a made-up example, imagine something needs to be refactored to a property. It could accidentially lead to bugs if the private field is visible to the whole class. #850 only addresses property-bound fields. This issue extends it to 'regions' in classes. BTW, nesting classes is ugly for the inteded use - before someone comes up with it ;-) Maybe @bondsbw is right that adding a |
In this specific case, I would focus on picking apart the suggested use examples. E.g. consider this refactoring: public class Person() //original syntax preserved
{
private _classField_1_; //original syntax preserved
private _classField_2_; //original syntax preserved
public Age Age { get; private set; }
public void CelebrateBirthday() => Age.IncreaseByYear();
/* rest of class and further blocks */
}
public sealed class Age
{
public Age(int years)
{
Years = years >= 0 ? years : 0; //use uint instead
}
public int Years { get; private set; }
public void IncreaseByYear() => Years += 1;
} I bet any example would most benefit from refactoring to extract new types / functions rather than the suggested language feature.
Like a |
@dsaf Besides, please, either silently correct the sample code or keep your comments for yourself! It is SAMPLE code, quickly written. And no, I do not want to use |
I like this proposal but the syntax of it leads to non-tight scopes e.g. lets say your existing class has: class Foo {
var {
int backing1_;
public int Backing1 { get { blah(this.backing1_); ... } }
}
var {
int backing2_;
public int Backing2 { get { blah(this.backing2_); ... } }
}
} And later on you want to add a method that needs access to backing1_ and backing2_. You now have to merge both scopes even though the Backing1 and Backing2 properties are unrelated. I think a better proposal would be something akin to how C++'s lambdas explicitly require you to list which member variables you can access within scope. e.g. class Foo {
int backing1_;
int backing2_;
public int Backing1 {
get {
using backing1_; // this lets us gain access to this.backing1_ within this scope
blah(this.backing1_);
}
}
public int Backing2 {
get {
using backing2_; // this lets us gain access to this.backing2_ within this scope
blah(this.backing2_);
}
}
} |
I feel like this proposal is driven by different goals than #850. Properties best fit a limited set of scenarios for simple field enhancements--like encapsulation and constraint enforcement--which represent a very common usage pattern. The goal of #850 is to further enhance those scenarios. This proposal is for deeper needs. But as pointed out by @dsaf, a separate encapsulating class is probably as good a tool. The syntax for a private nested class is little different from what you proposed. The reason @dsaf's class suggestion seems bloated is because the example in the original proposal is better achieved via the property. |
I get the feeling that I haven't made clear enough the purpose of this proposal. I'll try again Target groupI know that in many scenarios completely different approaches with different data structures or a completely different OOP design will lead to the same goal. But, C# is also used by thousands of hobby or part-time programmers, who realise their project without big teams and conceptual meetings. This proposal aims to those programmers. C# is multi-paradigm, not only big-data OOP! Q'n'D codeWith the target group in mind, many programs are written quick and dirty - even for production scenarios in SMBs. And as "quick and dirty" implies, this code may contain bugs due to some loose structures. Having easy coding technics, without completely refactoring the code on whiteboards, could lead to much more stable code and hinder bugs to occur. Especially when the codebase grows. Cost effectivenessYou don't need to use a sledgehammer to crack a nut. Very often a lightweight coding leads to the same resolution and is much, much quicker written - therefore cheaper (if that is of interest). Having a construct that improves stability in theese cases is preferable, again without a complete OOP refactoring. For this proposal, please keep theese three targets in mind. It targets tens of thousands of programmers who use C# on a more or less frequent basis, thus I think a relevant group. |
@DerpMcDerp How about this. Adding a new modifier class Foo {
local uint _age; // member declaration
local uint _previousAge;
public void CelebrateBirthday()
{
local _age, _previousAge; // "re-declaration", meaning these memebers are used in this block
_previousAge = _age++;
}
public void GetOlder(uint years) // obsolete method
{
_age += years; // compiler-error, member _age not locally declared!
}
} Another advantage is that this wouldn't break the existing API, because it is just another modifier. The IDE can easily help finding occurences to make sure that the members are only used in methods where intended. Obviously IntelliSense does not propose redeclared local members. The In case of a refactoring, a previously |
The problem with that entire concept is that the very target you intend is also the least likely to actually employ this feature. Someone who slops together a giant monolithic class full of interdependencies isn't going to take the time to whittle down those dependencies into concrete scoping blocks anymore than they're likely to refactor that giant class into smaller classes. C# shouldn't add features which encourage abuse of the language and incorrect design patterns. |
The
I would argue that it does.
Novice programmers don't care about scoping.
I did not know whether that syntax was a mistake or not.
Why not take time? Code is read more than it is written.
That's my point. This suggestion would benefit from a sample that demonstrates a practical benefit in addition to syntax. |
@dsaf
But I nevertheless guess that a class is much more overhead.
Contra, again ;-)
Sure, we are digital, there is nothing in between novice and pro programmers.... c'mon...
Sorry, I don't have time to take time ;-)
Well, I think that everybody who has done some coding actually knows where his real world samples are. This proposal would make sense in some mid-size classes (w/o refactoring them to 100 other related classes!). If you like a real world story: I have several projects in different languages. In some projects I started to build a nice OOP design on top of a running solution, just to refactor the code for the better. Very often the code grew and grew - still no real functional improvement. That took a lot of time. And often at some point I regreted to do so, because it got more and more complicated and I somehow lost control. I took it as a nice practice then. But the productive solutions today still runs on the old 'easy' code, with some improvements here and there. As it mostly points out when talking to others, in the real word there are many, many, many, many people who work the same as I do and who experience the same things. So instead of discussing about how something should be coded it would be better to stand on both feets, have a look in the real world and discuss whether a feature could be of real practical use to many of its potential users. It would still be a discussion, nevertheless. The latest
You see, it might be worth having a deeper look into this when considering that not all projects/programs are thoroughly planned on a whiteboard as learned on the college ;-) |
Another issue is with partial classes, e.g. partial class Foo {
var {
int foo;
}
}
partial class Foo {
// no way to access this.foo here
} v.s. partial class Foo {
local int foo;
}
partial class Foo {
// should we be able to access this.foo by using "local foo;"?
} |
With As But here we see, that the use of A better keyword might be explicit int foo = 1;
explicit void Bar(int Value);
public void DoIt()
{
explicit int foo; // this time without initialisation
explicit void Bar(int); // parameter name not important for signature
/* ... */
} [explicit Maybe someone, e.g. native english speaking, can come up with a better keyword that starts with p, so that it better fits in the row of member access modifiers like Again some words about Like with Python the IDE (and you also, hopefully) formats the code nicely with indention that respects the logical hierachy of things. Sibling members start on the same vertical column, more indented variables and (local) methods are not visible to the less indented ones (roughly, without respect to access modifiers). A further block on class level would introduce another level of indention, but its access decorated members would be visible for the less indented ones, though they are actually siblings in the class. That somehow uargh's me. Therefore I now think that the approach with additional class level blocks actually isn't appropriate. We should focus on the |
So for a proposal targeted at the audience of novices and hobbyists who aren't as familiar with proper programming guidelines, there is a lot of additional necessary syntax required in order to utilize the feature. Syntax that a novice or a hobbyist would not be very likely to know or understand how to use, let alone spend the time and energy planning out. This doesn't create a pit of success; it creates a mountain. |
I don't see much value in declaring the field in the class scope and then again in the method. It's confusing. It's bloated. I understand wanting to generalize #850 for use in methods, but being able to opt-in to accessing the field from anywhere else somewhat defeats the point. Perhaps #49 covers another way that neither causes more indentation nor adds extra declarations: public int IncrementRunningTotal()
{
static var runningTotal = 0; // Initialization only occurs the first time, per instance.
return runningTotal++; // The incremented value will be retained the next time
// this method is called on this instance.
} EDIT: I just realized that VB.NET's |
This should solve the scoping rules. Sorry if it has been described above in another way. public class Person : ReactiveObject
{
// public declarations inside the scope are also public outside the class
public scope {
// private is only available in the scope
private double _data;
// public is available outside the scope
public double Height {
get{ return _data;}
set{ this.RaiseAndSetIfChanged(ref _data, value);}
}
}
// public declaration inside the scope are visible through the class
// but invisible outside the class.
private scope {
private double _data;
public double Wealth {
get{ return _data;}
set{ this.RaiseAndSetIfChanged(ref _data, value);}
}
}
} It would also be nice to be able to factor out and reuse scopes. For example declare public scope Property<T> : ReactiveObject {
private T _data;
public T {{Name}} {
get{ return _data;}
set{ this.RaiseAndSetIfChanged(ref _data, value);}
}
} This declaration of Person is the same as the above declaration of person public class Person : ReactiveObject
with Property<double> as public Height
with Property<double> as private Wealth
{
} |
I'm not sure that I like this feature, but I want to point out that the current conversation has focused on the target being relatively novice developers, whereas the only real world usage given by the OP was more about refactoring messy code to better patterns. Maybe novice developers isn't actually the right use case here? |
@togakangaroo Thank you for pointing this out again! "about refactoring messy code to better patterns", exactly. And doing so without the effort of costly hours by whiteboarding complex design patterns. Just a quick, clean and lean refactor to reduce side effects and to prevent from accidentially building up new bugs in other places. @bradphelan |
public and private mean what they mean. How does replacing them with other words make it any more clear. A declaration is either public to a scope or private to a scope. If you chose to use "protected" and to make it consistent then it would mean that perhaps you could do scope inheritance and then protected declarations would become available to inheritors. However I can't really think a use case for scope inheritance. That's a step too far for me. |
Maybe it is better to let the analyzer check if a field belongs to a region of code, by using a preprocessor directive: #varregion
int _counter = 0;
public void Increase() { _counter++; }
public void Decrease() { _counter--; }
#endvarregion A preceding |
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 have not moved this feature request to the csharplang repo because I don't believe it would ever rise in priority over other requests to be something we would ever do in any particular release. However, you are welcome to move discussion to the new repo if this still interests you. |
Often there are fields in a class that are only used by a few methods or even only one property.
In addition to #850 I suggest adding block syntax for grouping fields to their associated methods only.
In this example
_age
won't clutter the IntelliSense and also generate a compiler error if used outside the block accidentially (by older code versions).It is important that only fields are bound to the block, not the contained methods, properties, subclasses, etc. To make that more verbose prepending the block by a keyword, like
var
, could be betterThe text was updated successfully, but these errors were encountered: