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: In-Class Namespaces #12849

Closed
hillin opened this issue Aug 1, 2016 · 23 comments
Closed

Proposal: In-Class Namespaces #12849

hillin opened this issue Aug 1, 2016 · 23 comments

Comments

@hillin
Copy link

hillin commented Aug 1, 2016

The Concept

In-Class Namespaces (ICN) would be useful to organize and categorize members for complex classes.
Example:

    class Character
    {
        namespace Basic
        {
            public double HitPoints { get; set; }
        }

        namespace Combat
        {
            public double ManaPoints { get; set; }
            public double Strength { get; set; }

            public virtual void OnDamaged(float damage) 
            { 
                this.Basic.HitPoints -= damage; // using a member declared in another ICN
            }

            public void Cast(SkillBase skill) { /* ... */ }
        }

        namespace Social
        {
            public void ReceiveMessage(string message) { /* ... */ }
            public void Say(string message)  { /* ... */ }

            namespace Interaction  // ICNs could be nested
            {
                public void Dance() { /* ... */ }
            }
        }

    }

Members can be accessed quite intrinsically:

    var character = GetNearestCharacter();
    character.Social.ReceiveMessage("hi!");
    /* OR */ character.Social::ReceiveMessage("hi!");

Overriding a method declared within a ICN:

    class InvincibleCharacter : Character
    {

        namespace Combat
        {
            public override void OnDamaged(float damage) 
            { 
                /* DO NOTHING, I'M INVINCIBLE */
            }
        }

        /* OR */

        public override void Combat::OnDamaged(float damage) 
        { 
            /* DO NOTHING, I'M INVINCIBLE */
        }

    }

This should be a language(compiler) feature, without requiring special IL representation or CLI support. For example, The Strength property under the Combat ICN of Character class, could be expressed as Combat.Strength (or Combat::Strength) in code, but it will be collapsed into a special property name in compile time, for example Combat$Strength, just like the backing field name of auto-implemented properties. This special name is also used in reflection to retrieve this property.

Why not regions, partial classes, or split it up?

ICN is a way of organizing class/struct members. Regions can do that, but they are internal - they only make sense to class authors, more like comments. Partial classes can do that too, but they are also internal.
For a user of an extensive class - which can be easily to appear in complex projects, when he typed a dot, he will be facing more than a hundred of members in the intellisense popup. Here we have a realistic example:

untitled

Here I'm finding an event in a WPF ListBox, with hundreds of members. While we are all used to it, isn't it possible to improve this situation? What if in the popup there is only a few categories - like Input, Layout, Appearance, Animation etc.? And you select Input, press Tab, all the input related properties, methods and events are listed now, make you much easier to identify the thing you really need.

This could be achieved by splitting the class in to several inner classes, like ControlInput, ControlLayout and so forth. But there are several problems:

  • It's tedious. You have to declare classes, put them into separated files to meet coding conventions, declare properties for their instances and instantiate them.
  • The instance of these classes require a reference to their owner, in order to access members placed in other inner classes.
  • Impossible or difficult to derive these inner classes, so you can't add or override members in them, for derived outer classes.
  • They are one-off classes, they won't be reused. It's somewhat like you have a long function DoSomething() and you say, no, this one is too long, I'm gonna split it up into DoSomething1(), DoSomething2() and DoSomething3().

In this scenario, inner class is not what you need semantically - and even if it is a choice, the tediousness discourages people to use it. That's why I'm proposing the ICN. It is essentially a way of adding prefixes to member names, in order to keep them categorized.

Original Post:

In-Class Namespaces (ICN) would be useful to organize and categorize members for big classes.
Example:

  class Settings
  {
      namespace General
      {
          public double Accuracy { get; set; }
          public bool DoubleCheckBeforeSave { get; set; }
      }

      namespace Audio
      {
          public double OutputLevel { get; set; }
          public double MicrophoneLevel { get; set; }

          public virtual void Calibrate() { /* ... */ }
      }
  }

Members can be accessed quite intrinsically:

  var settings = this.LoadSettings();
  settings.Audio.Calibrate();
  /* OR */ settings.Audio::Calibrate();

Overriding a method declared within a ICN:

  class DerivedSettings : Settings
  {
      public override void Audio::Calibrate()
      { 
          base.Audio.Calibrate();
          /* ... */ 
      }
  }

This should be a language feature, without requiring special CLI support. For reflection usage, the members in ICNs should be accessible with a certain naming rule, such as Audio::Calibrate.

@dsaf
Copy link

dsaf commented Aug 1, 2016

big classes

Split them!

@hillin
Copy link
Author

hillin commented Aug 1, 2016

@dsaf
We don't split a big class because it is simply big, we split it because it contains logically unrelated parts or reusable parts.

@hillin
Copy link
Author

hillin commented Aug 1, 2016

@dsaf
And this feature is also useful for not-that-big classes, so I revised the description :-)

@dsaf
Copy link

dsaf commented Aug 1, 2016

@hillin

We don't split a big class because it is simply big, we split it because it contains logically unrelated parts or reusable parts.

class GeneralSettings
{
    public double Accuracy { get; set; }
    public bool DoubleCheckBeforeSave { get; set; }
}

class AudioSettings
{
    public double OutputLevel { get; set; }
    public double MicrophoneLevel { get; set; }

    public virtual void Calibrate() { /* ... */ }
}

Not being picky, I always split config in my code: https://en.wikipedia.org/wiki/Interface_segregation_principle

@hillin
Copy link
Author

hillin commented Aug 1, 2016

@dsaf
Yes that's exactly what we are doing right now. But I think it's unnecessary to have so many setting classes and maintain a hierarchical structure manually, while all those classes are instantiated only once.

@paulomorgado
Copy link

@hillin,

What exactly are these ICNs you're proposing?

Are you aware that namespaces can only have types, not fields, properties, events or methods?

Are you aware that there is such a thing as inner types (types that you can declare inside other types)?

With inner classes, you can write it like this:

public class Settings
{
    public class GeneralSettings
    {
        public double Accuracy { get; set; }
        public bool DoubleCheckBeforeSave { get; set; }
    }

    public class AudioSettings
    {
        public double OutputLevel { get; set; }
        public double MicrophoneLevel { get; set; }

        public virtual void Calibrate() { /* ... */ }
    }

    public GeneralSettings General { get; protected set; }
    public AudioSettings Audio { get; protected set; }
}

and use it like this:

var settings = this.LoadSettings();
settings.Audio.Calibrate();

@hillin
Copy link
Author

hillin commented Aug 1, 2016

@paulomorgado

Literally, a namespace is a space where names could be placed, so these names could be separated from names in other namespaces. Currently, namespaces allow type names and inner namespace names inside of them; but why not member names? It's more of a way of categorizing things, not a replacement of nested classes.

@bradphelan
Copy link

@hillin is basically asking for singleton support. In C# such support is not very good. In Scala and Kotlin for example it's baked into the language.

https://kotlinlang.org/docs/reference/object-declarations.html

@svick
Copy link
Contributor

svick commented Aug 1, 2016

@hillin

But I think it's unnecessary to have so many setting classes and maintain a hierarchical structure manually, while all those classes are instantiated only once.

So, this is just to save few lines of code? Could you give an example where maintaining the structure as classes is bad and how ICN improve the situation?

@dsaf
Copy link

dsaf commented Aug 1, 2016

@bradphelan deliberate language-level support for anti-patterns is a novel concept for me!

@bradphelan
Copy link

bradphelan commented Aug 1, 2016

@dsaf You are mistaking immutable object singletons vs global mutable state as being an anti pattern. In well written scala most objects will be immutable. objects in scala can be scoped, are first class, can be passed around and tested.

But here's a reddit thread for you to argue your point on if you care as I don't think snide quips are useful here.

https://www.reddit.com/r/scala/comments/3o4bk2/why_does_scala_support_singleton_when_its/?st=irc01ade&sh=453464af

@dsaf
Copy link

dsaf commented Aug 1, 2016

@bradphelan I think Scala and Kotlin naturally benefit from above-average developers attracted to newer languages. I have seen enough poor-quality C# code to be aggressive towards features that facilitate such code.

@paulomorgado
Copy link

@hillin, namespaces are just parte of the name of the type. They don't have a physical existence. That's why this:

namespace N1
{
    namespace N2
    {
        namespace N3
        {

is the same as this:

namespace N1.N2.N3
{

And only types (class, struct) have namespaces. Fields, properties, events or methods don't.

I can't even imagine what you're proposing means in terms of IL.

@vbcodec
Copy link

vbcodec commented Aug 1, 2016

Maybe other option is to allow to use dots in function's names, and modify intelisense to use these dots for grouping just like for namespaces

@svick
Copy link
Contributor

svick commented Aug 1, 2016

@paulomorgado I think the intention is that, just like namespace N { class C {} } actually creates a class named N.C, class C { namespace N { public double Accuracy { get; set; } } } would create a property named N.Accuracy in the class C.

(But just because I can imagine what it would mean in IL does not mean I think it's a good idea.)

@alrz
Copy link
Member

alrz commented Aug 1, 2016

It seems that you don't actually need scoping. For merely organizing code, I'd suggests using #region or partial.

@hillin
Copy link
Author

hillin commented Aug 2, 2016

Thanks everyone.

Please allow me explain this idea more clearly.

Basically, I'm proposing a way of organizing class/struct members. Regions can do that, but they are internal - they only make sense to class authors, more like comments. Partial classes can do that too, but they are also internal.
Setting class might not be quite a good example, because its parts are none-to-loosely related. Think about a Character class in an RPG game:

    class Character
    {
        #region Basic Attributes
            public double HitPoints { get; private set; }
            public double ManaPoints { get; private set; }
            /* ... and 10 more attributes */
        #end region

        #region Combat Attributes
            public double Strength { get; private set; }
            public double Agility { get; private set; }
            /* ... and 10 more attributes */
        #end region

        #region Combat Methods
            public void OnDamaged(double damage) { /* ... */ }
            public void Cast(SkillBase skill) { /* ... */ }
            /* ... and 20 more methods */
        #end region

        #region Social Methods
            public void ReceiveMessage(string message, IMessageSource source) { /* ... */ }
            public void Say(string message)  { /* ... */ }
            /* ... and several other methods */
        #end region
    }

It's quite easy to have such extensive classes in complex projects. In the example above, I used multiple regions to organize the code - in real scenarios, I also prefer to split these code into several partial classes, and edit the csproj file manually to create a tree hierarchy in the solution explorer.
But for a user of this class, when he typed a dot, he will be facing more than a hundred of members in the intellisense popup. While we are all used to it, isn't it possible to improve this situation? What if in the popup there is only a few categories - like Combat, Social, Input, Audio etc.? And you select Combat, press Tab, all the combat related properties and methods are listed now, make you much easier to identify the thing you really need.

This could be achieved by splitting the class in to several inner classes, like CharacterCombat, CharacterSocial and so forth. But there are several problems:

  • It's tedious. You have to declare classes, put them into separated files to meet coding conventions, declare properties for their instances and instantiate them.
  • The instance of these classes require a reference to their owner, in order to access members placed in other inner classes.
  • Impossible or difficult to derive these inner classes, so you can't add or override members in them, in derived classes.
  • They are one-off classes, they won't be reused. It's somewhat like you have a long function DoSomething() and you say, no, this one is too long, I'm gonna split it up into DoSomething1(), DoSomething2() and DoSomething3().

In this scenario, inner class is not what you need semantically - and even if it is a choice, the tediousness discourages people to use it. That's why I'm proposing the ICN. It is essentially a way of adding prefixes to member names, in order to keep them categorized. It don't need a representation in IL and special support in CLI, because it's a compiler feature. For example, The Strength property under the Combat ICN of Character class, could be expressed as Combat.Strength (or Combat::Strength) in code, but it will be collapsed into a special property name in compile time, for example Combat$Strength, just like the backing field name of auto-implemented properties.

I'm also revising my original proposal to employ a better example.

@hillin
Copy link
Author

hillin commented Aug 2, 2016

@dsaf @paulomorgado @bradphelan @svick @vbcodec @alrz @Pilchie
Guys sorry to tag you all, but I've revised my main post to better reflect my idea. It would be appreciated if you can read it again.

@stepanbenes
Copy link

stepanbenes commented Aug 2, 2016

So the problem is hundreds of members in intellisense popup that is facing the user of your class. This can be addressed by better tooling. E.g. Intellisense window could understand regions inside class and group the members according to region names.

@dsaf
Copy link

dsaf commented Aug 2, 2016

I have a mixed feeling about this. I appreciate the effort you are putting in and also accept that there might be slightly different personal styles of programming. However I feel that it goes against SOLID principles.

It's tedious. You have to declare classes, put them into separated files to meet coding conventions, declare properties for their instances and instantiate them.

I actually agree with the first part. Sometimes it feels weird to put a small interface with single implementation into a separate file. This is a problem that would be nice to address somehow. E.g. F# is not afraid to put a lot of stuff into one file. Maybe the shorter C# 8 syntax will help?

The second part is just a nature of decoupled code.

The instance of these classes require a reference to their owner, in order to access members placed in other inner classes.

Excuse me, but it sounds like a problem with the design of the code. If your project is a game, have you considered this pattern http://gameprogrammingpatterns.com/component.html?

Impossible or difficult to derive these inner classes, so you can't add or override members in them, for derived outer classes.

I guess this is a nature of inner classes.

They are one-off classes, they won't be reused. It's somewhat like you have a long function DoSomething() and you say, no, this one is too long, I'm gonna split it up into DoSomething1(), DoSomething2() and DoSomething3().

Not sure if I understand this correctly, but I absolutely would split a long function.

@alrz
Copy link
Member

alrz commented Aug 2, 2016

In fact, this problem have been addressed with #region and partial classes in the declaration site, however,

@hillin: But for a user of this class, when he typed a dot, he will be facing more than a hundred of members in the intellisense popup.

In this case, it is indeed a tooling issue, agreeing with @stepanbenes. Except for that regions wouldn't be available in a compiled assembly, supporting something like CategoryAttribute would do though.

@svick
Copy link
Contributor

svick commented Aug 2, 2016

I still think inner classes are good enough of a solution:

It's tedious. You have to declare classes, put them into separated files to meet coding conventions, declare properties for their instances and instantiate them.

It is somewhat tedious, but your use case is complex classes. If you really have hundreds of members, then the few dozen lines of tedious code are not a big issue.

And I don't think language should adapt to coding conventions. If your coding conventions prevent you from writing good code, you need to change the conventions, not change the language.

The instance of these classes require a reference to their owner, in order to access members placed in other inner classes.

Why is that a problem? Just because it's more tedious code?

Impossible or difficult to derive these inner classes, so you can't add or override members in them, for derived outer classes.

It would require some generics gymnastics, but I wouldn't call it difficult, certainly not impossible.

They are one-off classes, they won't be reused. It's somewhat like you have a long function DoSomething() and you say, no, this one is too long, I'm gonna split it up into DoSomething1(), DoSomething2() and DoSomething3().

I think classes (or methods) that are not reused are fine. If a method is too long and can be logically split into multiple methods, then I would split it, because it makes the code more readable.

@gafter
Copy link
Member

gafter commented Sep 11, 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 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.

@gafter gafter closed this as completed Sep 11, 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