Improve Namespaces and Static Class Methods

[shaggygi]

Update: There was some input regarding static methods and probably not the best approach, so disregard thoughts specific to static.

This discussion attempts to point out a few areas related to current namespaces and classes. Understanding there have been similar discussions previously, but IMHO wanted to try and combine a few notes to hopefully improve the toolkit.

While having the VS class simplifies ease of access, it seems like the APIs could be improved by updating the namespaces and changing most of the sections to use static methods (where applicable). Below uses the StatusBar as an example, but is also relevant to others like Solutions, Documents, etc.

More Detailed Namespaces

First, the namespace should follow a similar path where located in folder structure (at least in most cases). This helps separate out the sections instead of dumping everything within Community.VisualStudio.Toolkit. It will get messy fast as you can already see by using IntelliSense with the toolkit.

For StatusBar, its namespace should be Community.VisualStudio.Toolkit.Notifications since it is located in the Notifications folder. It would look like the following as a result:

namespace Community.VisualStudio.Toolkit.Notifications
{
    /// <summary>
    /// An API wrapper that makes it easy to work with the status bar.
    /// </summary>
    public class StatusBar
    {
        //... code
    }
}

Improved Class Naming

There have been some earlier discussions on naming of various toolkit classes. I still believe it would be appropriate to rename StatusBar to StatusBarNotification even if it is within the Community.VisualStudio.Toolkit.Notifications namespace. I agree it is somewhat redundant, but will not appear that way when using a using statement and having the class name by itself. Use Syntax from the Roslyn repo as example where they have "Syntax" in the name.

It would look similar to the following as a result:

namespace Community.VisualStudio.Toolkit.Notifications
{
    /// <summary>
    /// An API wrapper that makes it easy to work with the status bar.
    /// </summary>
    public class StatusBarNotification
    {
        //... code
    }
}

Changing to Static Class Methods

All methods currently in the StatusBar class can be changed to static as it doesn't include anything needed to instantiate the object first. As an example, you would have the following static method:

/// <summary>Gets the current text from the status bar.</summary>
public static async Task<string?> GetMessageAsync()...

Combining Points Above

Combining the changes above can simplify the toolkit for readability, usage, and maintenance. As an example, you don't necessarily have to use the VS class.

// Before
using Community.VisualStudio.Toolkit;

await VS.StatusBar.ShowMessageAsync("Have a nice day!");

// After
using Community.VisualStudio.Toolkit.Notifications;

await StatusBarNotification.ShowMessageAsync("Have a nice day!");
// or include full namespace if you wish...
await Community.VisualStudio.Toolkit.Notifications.StatusBarNotification.ShowMessageAsync("Have a nice day!");

Final Thoughts

Of course, these are just my simple opinions. These suggestions are not specific to the current VS class (just used as an example), but more about improving the overall structure of the toolkit's APIs. As noted, the example changes for the StatusBar class could also be applied to other areas like InfoBar, Documents, and Solutions. Any feedback/thoughts are appreciated in this collaborative effort. I don't mind attempting a PR if the above seems like a good approach. Thanks for reading.

[reduckted]

All methods currently in the StatusBar class can be changed to static as it doesn't include anything needed to instantiate the object first.

Using static methods makes it very difficult to unit test codes that uses those methods, because you can't mock them. For example, let's say my extension should show the progress of some operation in the status bar. If I want to verify that my extension is displaying the progress correctly, I could mock the StatusBar class (see #85) and look at the calls to the ShowProgressAsync method. I could then verify that the method was called with the correct currentStep and numberOfSteps values.

If the ShowProgressAsync method was static, I wouldn't be able to see what arguments the method was called with, which leaves me with no way of verifying that the progress is being calculated correctly.

Related discussion about using interfaces for major types: #85

[shaggygi]

@reduckted that is a good point and agree. I had read #85, but misunderstood thinking there were not going to be interfaces added. That said, the static method section should be disregarded, but believe the other points still seem valid. Thx for the feedback 👍