Skip to content
This repository was archived by the owner on Jul 12, 2022. It is now read-only.
This repository was archived by the owner on Jul 12, 2022. It is now read-only.

Should convert "static readonly" field to Pascal style? #281

@AllenLius

Description

@AllenLius

A "static readonly" field more like a const field that can be changed only in static ctor.

Activity

lindexi

lindexi commented on Nov 28, 2018

@lindexi
Member

Yes

AllenLius

AllenLius commented on Nov 28, 2018

@AllenLius
Author

Yes

But I see the "static readonly" fields is converted to camel style with '_' prefix:
"static readonly Dictionary<string, int> _captureIndex".

stephentoub

stephentoub commented on Nov 28, 2018

@stephentoub
Member

public static fields should be PascalCased.
internal and private static fields should be s_camelCased with the "s_" prefix.

AllenLius

AllenLius commented on Nov 28, 2018

@AllenLius
Author

public static fields should be PascalCased.
internal and private static fields should be s_camelCased with the "s_" prefix.

And I think "static readonly" should be PascalCased too instead of _camelCased in current behavior of the tool.

stephentoub

stephentoub commented on Nov 28, 2018

@stephentoub
Member

And I think "static readonly" should be PascalCased too instead of _camelCased in current behavior of the tool.

I don't know what the current behavior of the tool is, but readonly should not influence the naming, at least as far as our style in corefx is concerned.

AllenLius

AllenLius commented on Nov 28, 2018

@AllenLius
Author

I don't know what the current behavior of the tool is, but readonly should not influence the naming, at least as far as our style in corefx is concerned.

For rule "12: We use PascalCasing to name all our constant local variables and fields".
A static readonly field just like a special const field that can be initialized inline or in the constructor method only, so I think we should use Pascal rule too.

stephentoub

stephentoub commented on Nov 28, 2018

@stephentoub
Member

A static readonly field just like a special const field that can be initialized inline or in the constructor method only, so I think we should use Pascal rule too.

No, there's a difference between consts and static readonlys, and we have different naming rules for them.

lindexi

lindexi commented on Nov 29, 2018

@lindexi
Member

@stephentoub as the style say it doesn't matter if you use or not to use an underline and whether it is public or not.

bartonjs

bartonjs commented on Nov 29, 2018

@bartonjs
Member

@lindexi I'm not sure I follow.

  1. We use _camelCase for internal and private fields and use readonly where possible. Prefix internal and private instance fields with _, static fields with s_ and thread static fields with t_. When used on static fields, readonly should come after static (e.g. static readonly not readonly static). Public fields should be used sparingly and should use PascalCasing with no prefix when used.

This means the naming rules are:

  • private Foo _foo;
  • private static Foo s_foo;
  • [ThreadStatic] private static Foo t_foo;
  • private readonly Foo _foo = InitializeFoo();
  • private static readonly Foo s_foo = InitializeFoo();
  • public Foo Foo;
  • public static Foo Foo;
  • [ThreadStatic] public static Foo Foo;
    • This can only go poorly 😄.
  • public readonly Foo Foo = InitializeFoo();
  • public static readonly Foo Foo = InitializeFoo();

The readonly-ness doesn't change the casing style or prefixes. public is what does.

While static readonly int is vaguely const-ish, static readonly Dictionary<A,B> isn't; readonly means "may not be reassigned after the constructor", which is not the same as C# const, C const or an immutable concept.

And we definitely wouldn't call private static readonly object s_lock = new object(); const, but it better be readonly, since reassignment would break anything locking on it.

AllenLius

AllenLius commented on Nov 29, 2018

@AllenLius
Author

@lindexi I'm not sure I follow.

  1. We use _camelCase for internal and private fields and use readonly where possible. Prefix internal and private instance fields with _, static fields with s_ and thread static fields with t_. When used on static fields, readonly should come after static (e.g. static readonly not readonly static). Public fields should be used sparingly and should use PascalCasing with no prefix when used.

This means the naming rules are:

  • private Foo _foo;

  • private static Foo s_foo;

  • [ThreadStatic] private static Foo t_foo;

  • private readonly Foo _foo = InitializeFoo();

  • private static readonly Foo s_foo = InitializeFoo();

  • public Foo Foo;

  • public static Foo Foo;

  • [ThreadStatic] public static Foo Foo;

    • This can only go poorly 😄.
  • public readonly Foo Foo = InitializeFoo();

  • public static readonly Foo Foo = InitializeFoo();

The readonly-ness doesn't change the casing style or prefixes. public is what does.

While static readonly int is vaguely const-ish, static readonly Dictionary<A,B> isn't; readonly means "may not be reassigned after the constructor", which is not the same as C# const, C const or an immutable concept.

And we definitely wouldn't call private static readonly object s_lock = new object(); const, but it better be readonly, since reassignment would break anything locking on it.

You are right, the readonly doesn't influence the name style.

An exception is that the private const field should be in PascalCased:
private const int Port = 80;
I think it is just a little tricky if we what to read the port from a config file instead:
private static readonly int s_port = GetHttpPort();
then we need to update the name in code.

lindexi

lindexi commented on Nov 29, 2018

@lindexi
Member

@AllenLius But I think the const int and static readonly has the same means.

AllenLius

AllenLius commented on Nov 29, 2018

@AllenLius
Author

@AllenLius But I think the const int and static readonly has the same means.
Yes, I'm on your side, the means for a dev is just the same: initialized once and should not be changed after that.
but the codeformatter is just follow the coding style of the donetcore.

bartonjs

bartonjs commented on Nov 29, 2018

@bartonjs
Member

const values are replaced by the compiler with their constant value, so if you look at generated output IL you'd see the literal 80.

static readonly means there's still a field-read. If a debugger, or unsafe code, changed the static field value, that would have a runtime effect on all places that read it.

A subtle difference, but a difference nonetheless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @stephentoub@AllenLius@bartonjs@lindexi

        Issue actions

          Should convert "static readonly" field to Pascal style? · Issue #281 · dotnet/codeformatter