Windowscommunitytoolkit: Review namespaces and folder structure coupling

[mrlacey]

Describe the problem this feature would solve

The way namespaces are currently used across the codebase is inconsistent.

This is a follow-on from the discussion started around #3405

Describe the solution

Ideally, there should be a standard that is defined for how namespaces are used. This should be documented.
The existing code should then be updated to this standard.

I propose that a single namespace should be used by each package unless there is a need or strong reason not to, or it is necessary for compatibility with external standards (such as the attribute classes being in the System.Diagnostics.CodeAnalysis namespace.
Using multiple namespaces when they are not required can lead to confusion on the part of the developer consuming them.
Having to use multiple namespaces when they are not necessary also adds overhead to the consuming code (even if just in adding using directives).
VS Tooling is able to infer the desired namespaces and offer suggestions to add them but they don't add any value when all they do is reflect the folder structure of the underlying source.

This may result in some breaking changes and so should be done as part of a major release.

Describe alternatives you've considered

Live with the inconsistency that currently exists.

Additional context & Screenshots

From a quick scan across the codebase, different projects/packages typically either use one namespace of everything or have lots of namespaces that reflect the folder structure.

Some noted from my quick review:

• Parsers have great inconsistency, especially in Core and MarkDown folders
• files in Services/Core folder inconsistently use the core namespace
  
  
  
  • One of the Weibo files has repeated text in the namespace
  
  
  • All the "UWP.Helpers" have lots of subdirectories but only use a single namespace
  
  
  • Adaptive/AdaptiveHelper.cs is in the M.T.Uwp.Notifications.Adaptive namespace but all other classes in that folder are in the M.T.Uwp.Notifications namespace
  
  
  • Classes in the Adaptive/Elements folder are in their own namespace but most of the other NOtifications classes are in the main namespace
  
  
  • WebViewExtensions.cs is in a WebVIew subdirectory but not in the Extensions namespace
  
  
  • UWP.UI/Helpers.DesignTimeHelpers isn't in the Helpers namespace but everything else in the folder is
  
  
  • UWP.UI/Animations has lots of namespaces
  
  
  • most of UWP.UI.Controls are in a single namespace apart from the MarkdownRenderer & TextToolbar
  
  
  • DataGrid.Utilities/DoubleUtil.cs seems to be missing a .UI. form the namespace
  
  

The above is based on a brief review of the current code.
I'd be happy to take a fuller look at all the code if further investigation of this issue and adopting a consistent convention is agreed upon.

Note also that the HighPerformance and MVVM packages are some of the biggest users of multiple namespaces. As there are the newest packages, changes in them may be impactful. However, if changes are to be made to the MVVM package it will be better to do this as soon as possible.

[msftbot[bot]]

Hello, 'mrlacey! Thanks for submitting a new feature request. I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

[Sergio0694]

I'm all for fixing the inconsistencies you mentioned, like:

• UWP.UI/Helpers.DesignTimeHelpers isn't in the Helpers namespace but everything else in the folder is
• most of UWP.UI.Controls are in a single namespace apart from the MarkdownRenderer & TextToolbar

I agree that there shouldn't be weird situations with single items breaking the pattern used in that package, or being the only exception compared to all other types in that folder or in that parent namespace, that sounds great! 👍

That said, I disagree with this point though:

I propose that a single namespace should be used by each package

Personally I like leaving VS/ReSharper to deal with the proper suggestions, and the fact that when you have multiple namespaces you get a much cleaner IntelliSense window, with only the APIs you're actually working with being displayed on top. It helps a lot especially when you have many APIs in scope, and it just looks cleaner in general.

Additionally, the namespaces often do indicate some conceptual categorization for APIs. Like, I could see all the controls in the Microsoft.Toolkit.Uwp.UI.Controls package being in that root namespace, because they're - well - controls. But if eg. you take the base package, the Guard APIs being in a separate namespace than eg. the collection interfaces makes perfect sense. Or, the HighPerformance having different namespaces for extensions, or buffers, makes sense too. That's the same categorization used in the BCL as well.

I mean, just try to use any API in the BCL right now - there's lots of different namespaces there, and it makes perfect sense. It's not like eg. the whole System.Runtime directory has a single namespace, there's ton of them. Same for other namespaces like System.Buffers - they didn't just throw all the new buffer types directly into System.

I think we should follow the same approach here.

Also to quickly touch upon the MVVM package in particular - I think keeping the separation helps both in mirroring the same namespaces from the BCL, as well as reinforcing the idea that we have loosely coupled components that can be used independently. If you don't need them, just don't include the namespace and you won't even be bothered by those other types in IntelliSense at all. That's why eg. I wouldn't personally want to use a single namespace in that package, for instance.

[michael-hawker]

Thanks @mrlacey! This is definitely something we should address alongside the dependency clean-up already planned for 7.0. I've added this issue for the namespace discussion specifically to our planning issue.

I definitely think there's some opportunities for things like Panels to move to their own package, though they may also live under the Controls namespace as they are generally all used together at times (at least in the system XAML they're all part of the same namespace).

It'd be good to get some input on best practices from a few different places. Do you know if any other OSS projects like .NET itself have any of these guideline documents already?

In the meantime, if you want to look for/list any other spots of inconsistency here, that'd be immensely helpful.

Thanks! 🦙❤

[mrlacey]

I definitely think there's some opportunities for things like Panels to move to their own package, though they may also live under the Controls namespace as they are generally all used together at times (at least in the system XAML they're all part of the same namespace).

Redefining what goes in separate packages should probably be a separate issue as there's lots of opportunity for discussion about what should go where.
Do you have real-world usage data to help back up decisions about what should go where? For example, are there lots of people only using "Panels" and no other controls? or is there another benefit of putting these in a separate package?

[jamesmcroft]

Namespaces are definitely important and some packages definitely won't warrant everything being bundled under the same namespace. It defines a clear separation between components and ultimately allows developers to access the bits they need from those.

Obviously for some parts and this leads into a different convo, is having smaller more discrete packages for a more "pick and choose" approach which reduces the size of apps or packages by allowing developers to only pull in the bits they need. That obviously helps reduce the number of namespaces in a single project.

The downside to that, of course, is knowing exactly what is in each package so you can make the decision to take the dependencies.

[jamesmcroft]

I think looking at the associated PR which was closed around the new MVVM package, I think there are some cases where it might make sense for things to be less separated out.

For example, ObservableObject and ObservableRecipient are core to the MVVM toolkit. You might expect those at the root. For the rest though, these are what I'd class as additional components. You've no need to use Messenger, IoC, or the commands if you don't need to. These being clearly separated by namespaces makes a lot of sense.

I could probably see the change in the additional namespace for messages under messaging as being a little confusing as all those components related to messaging in general.

There's no right or wrong way to this though. That being said, I definitely wouldn't create an application and have every class in it at the root namespace.

[mrlacey]

It'd be good to get some input on best practices from a few different places. Do you know if any other OSS projects like .NET itself have any of these guideline documents already?

.NET Framework design guidelines for namespaces

However, acknowledging that rules for a large framework don't automatically apply to a _much_ smaller library.

StackOverflow has lots of questions asking about how to structure/name namespaces and almost all answers point back to the framework guidelines.

The only projects I can find with any specific rules about namespaces in contribution guidelines either say "put everything in one namespace" or "There are only two namespaces (X & Y.) Us X for xxx and Y for yyyy."

I suspect that this project is in a unique position regarding this problem as multiple contributors have done different things over periods of time and no-one has looked at addressing/standardizing this previously.

Interesting note: WinUI only uses 8 namespaces.

[Sergio0694]

"For example, ObservableObject and ObservableRecipient are core to the MVVM toolkit. You might expect those at the root."

@jamesmcroft The rationale here was to closely mirror the BCL namespaces to reflect the fact that we're providing "reference implementations" for the interfaces there. I've mentioned this in a previous reply to @mrlacey, in general it goes like this:

• Microsoft.Toolkit.Mvvm.ComponentModel ---> System.ComponentModel (INotifyPropertyChanged)
• Microsoft.Toolkit.Mvvm.Input ---> System.Windows.Input (ICommand)
• Microsoft.Toolkit.Mvvm.Messaging ---> well there's no messenger in the BCL 😄
• Microsoft.Toolkit.DependencyInjection ---> Microsoft.Extensions.DependencyInjection (for IServiceProvider)

Additionally, I think having all the components in a separate namespace would reinforce the idea that this library is really pick and choose. That's to say, some devs might very well want to use it just for eg. the Ioc class, or the commands. Having the ObservableObject etc. classes right in the root might instead suggest that you have (or should) use them, and that just the other components are really meant to be optional, which is not the case. Again this is my personal take on this, hope it makes sense! 😊

That said, I guess I could see an argument being made to move the messages into the parent namespace, even though I really do like the additional separation there. The way I see it, the main Messaging namespace includes functionality (so the messenger itself, the interface, and the extensions), while the sub-namespace includes data items, ie. the message themselves. Which in turn are optional - you might very well want to just use the messenger with your own custom messages, in which case you wouldn't want all those extra types popping up in IntelliSense right next to the APIs you actually want to use.

@mrlacey As far as examples from other large projects, the repo I often take inspiration from when organizing code is ImageSharp, which I think is simply one of the best .NET codebases in existance, it's just incredible. You can see that they have a very namespace heavy approach too, which closely (almost perfectly) mirrors the folder structure too. I've been contributing to that repo since last year and I've followed many of the issues and conversations there - so far I don't recall ever seeing anyone complaining about having too many namespaces there.

I'd say especially with even vanilla VS being so good at automatically including namespaces today, there shouldn't really be a problem with this either, and I much rather prefer having more separation in the included APIs when adding a reference 👍

[mrlacey]

I mean, just try to use any API in the BCL right now - there's lots of different namespaces there, and it makes perfect sense. It's not like eg. the whole System.Runtime directory has a single namespace, there's ton of them. Same for other namespaces like System.Buffers - they didn't just throw all the new buffer types directly into System.

• What makes sense for a large framework such as the BCL doesn't automatically apply to a small library.
• Many part of the larger namespaces in System are divided into separate sub-namespaces as they are distributed in different assemblies.

I'm not convinced your above arguments apply here.

[Sergio0694]

"What makes sense for a large framework such as the BCL doesn't automatically apply to a small library"

I never claimed it _automatically_ applies to a small library, I said I think it makes sense for these toolkit packages 😊

And yeah I know some of the System APIs are distributed in separate packages, but even them many of those include APIs from a number of different namespaces, as they directly map to the original APIs in the BCL. To make an example, the System.Memory package includes APIs from System, System.Buffers, System.Runtime.CompilerServices.Unsafe, etc. Same for other packages that can also be installed separately.

That said, I'm not trying to argue that it's a good idea to do this in the toolkit just because the BCL does something similar. I did mention other points to explain why I think it makes perfect sense for us to have more namespaces in many toolkit packages.

In general I don't think we should prioritize "convenience" in having direct access to all the APIs in exchange for a better conceptual separation between APIs, especially with IntelliSense being able to automatically search and add the necessary using directives on its own today. I would say the development experience is already extremely convenient as is thanks to the way IntelliSense and autocompletion work, and this way you also get better code separation on top - the best of woth worlds in my opinion 👍

[mrlacey]

I think there are two potentially conflicting positions that are being argued for here.

1. What's best for (or preferred by) the authors of the toolkit code.
2. What's best for the people consuming the toolkit libraries in their apps.

In an ideal world these align but that's not always the case.

_Having lots of knowledge about the code (due to writing it) makes it incredibly different to imagine the consumption of the APIs by someone not familiar with it._

---

How does "a better conceptual separation between APIs" benefit the people using the library? You say it's a good thing (and at an abstract level I agree) but how is it good for the consumers of the library?

In general I don't think we should prioritize "convenience"

😱 😱 😱 😱 😱 😱 😱 😱
I absolutely think the opposite!
The toolkit exists to provide "helpers" and "simplify" development.
"It just works" should be a goal.
It's not a goal to force developers to follow (or event know) specific development patterns or practices.
It is not a goal to force people to understand the underlying structure or how classes relate to interfaces in the BCL.

My initial experience porting existing code to the new framework was that it wasn't a lot of effort to do it but there were lots of times where it didn't "just work".
Within each class I had to add two or three namespaces but they were all related to using the MVVM library. There were plenty of times where my inner monologue was:
"Why doesn't it know about RelayCommand here?"
"Oh, I need to add another reference."
"But I'm in a ViewModel that references ObservableObject already"
"Oh, it's a different namespace"
"I guess I'll just let it add what it suggests."

There was no "it just works" I had to rely on the tooling to make it work.

Having the authors of the toolkit code ensure that classes are loosely coupled by using multiple namespaces is one thing.
But once the code is written, the existence of multiple namespaces does nothing to enforce the coupling of classes. It just means that there are more namespaces that the consuming code needs to reference. Doesn't it?

---

Having lots of using directives in a file is a code smell that indicates a file (and the class it contains) may be doing lots of different things. In turn this suggests the code might be better being divided up because it's trying to do too much.
Adding lots of using directives that are all related to a narrowly defined area makes the code look more complicated than it is.

---

The arguments for multiple namespaces being good/ok because the VS tooling can suggest missing namespaces seem contradictory.

• Not wanting to fill the autocomplete list with unnecessary extra entries is a good aim but as VS _can_ suggest things that aren't in referenced namespaces this doesn't hold true.
• Trying to use a class from a namespace that isn't referenced means that there is a greater need for the developer to know and type the full name and not rely on intellisense/autocomplete to minimize keystrokes.
• Needing to use something that isn't in a referenced namespace also limits discoverability and enforces a need to read the documentation and remember what's available.

[Sergio0694]

Not to nitpick, but I think you're strawmanning my position here:

In general I don't think we should prioritize "convenience"

😱 😱 😱 😱 😱 😱 😱 😱
I absolutely think the opposite!
The toolkit exists to provide "helpers" and "simplify" development.
"It just works" should be a goal.

My use of quotes around "convenience" was deliberate, but you replied to that as if I hadn't used them at all.

I did _not_ say that we should not prioritize convenience, but that we should not prioritize "apparent" convenience (hence the quotes). What I mean by this is - you can _already_ access all the types anyway. IntelliSense will already search and suggest all the types across all the existing namespaces in each referenced assembly regardless of the namespace they're from. You literally only need to press on key for autocomplete to kick in, type the rest of the name and also add the using directive for you.

In addition to this, you also get:

• Better conceptual separation for components that logically belong to different areas, and that do different things
• Less clutter in IntelliSense when you're only working with APIs only related to a specific category, as the ones you haven't imported will be displayed further down in the IntelliSense window, under the list including those that don't have a using directive yet (but that are still indirectly available as part of a referenced assembly).

And I really, really value this conceptual separation here, especially when you're working in projects that have a number of package references in them. If each package just puts everything in the root namespace because it's "convenient" (in quotes), then the IntelliSense window quickly gets out of hand as you start adding even just a few different using directives. And then again, even just the logical separation of categories just makes sense on its own too, for a number of reasons.

As to your experience porting code, you did mention that your case was pretty unique working on the template studio, having to deal with an incredible number of configurations and a very peculiar template generation system. I don't think that reflects the typical use case for the toolkit. Eg. just as an example, I mentioned I ported my app and it was a very easy and painful transition - the different namespaces didn't really cause any slowdown at all.

[mrlacey]

@Sergio0694 Given that you're the author of the MVVM package you get the final say in how it's structured. I'll agree to differ with your preferences.
In terms of the wider toolkit, are there any broad guidelines that can be applied, or should this issue just be about addressing inconsistencies and anomalies in the current codebase?

[Sergio0694]

Just to reiterate, while I disagree with you on some points mentioned, I do appreciate the conversation! 😊

And I absolutely agree on fixing all the various inconsistencies/anomalies across packages, for starters 👍

[Nirmal4G]

My suggestion is that we should follow the existing Windows Runtime APIs naming conventions. There should be internal docs on best practices. Since Windows Toolkit extends UWP, I think opening up the dev docs within Project Reunion and extend them here as per our needs.

No need to reinvent the wheel. Personally, we should straight up ask either @terrajobst or Framework Design Guidelines for help. 😉