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

Why shouldn't parameters be public? #15485

Closed
tidyui opened this issue Nov 9, 2018 · 1 comment
Closed

Why shouldn't parameters be public? #15485

tidyui opened this issue Nov 9, 2018 · 1 comment
Labels
area-blazor Includes: Blazor, Razor Components

Comments

@tidyui
Copy link

tidyui commented Nov 9, 2018

I'm building a composite application where you register the different fields your data-models can use at startup.

RegisterField<T>() where T : IField

As a result you also need to register the component that should handle the field in the Blazor App. To make sure that the component provides that basic functionality needed by the generic part of the app I want the component to also implement an interface using the @implements keyword.

RegisterFieldComponent<T> where T : IFieldComponent

Naturally I want to include the mandatory parameters these components need to have for them to function but then the compiler throws a warning telling me that parameters shouldn't be public. As I can't get a good night sleep with compiler warnings in my projects this only leaves with the option to use an abstract base class instead of an interface but this feels a bit dodgy as I'm not really encapsulating any logic, I only want the components to fulfill a certain contract.

What is the reason for the warning that parameters shouldn't be public? Help me get a good night sleep with a good motivation 😀

Best regards

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Nov 9, 2018

To have a clear mind about this, you have to recognize the distinction between parameters, which are how data flows down the component hierarchy and is integrated into the rendering system, and properties, which are just a C# concept and have no way of interacting directly with the rendering system.

  • You can make your properties have whatever access rules you want. They are just C# properties. Do what you like.
  • For anything you declare as a parameter (by adding [Parameter]), you're saying it's an aspect of the rendering system. It should only be mutated by the rendering system, or you'll get out of sync and confused.

So, I think we should tweak the analyzer rule to have it warn only if the parameters are publicly settable. There's no problem with them being publicly readable. They just shouldn't be publicly writable (because if they are, you're inviting others to mutate them, even though this will get you out of sync and make your app seem buggy).

In your specific example, an abstract base class sounds like a good fit. An interface wouldn't be sufficient because it can't enforce the presence of [Parameter].

@mkArtakMSFT mkArtakMSFT transferred this issue from dotnet/blazor Oct 27, 2019
@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Oct 27, 2019
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

No branches or pull requests

3 participants