miniscript icon indicating copy to clipboard operation
miniscript copied to clipboard

Modernizing C# code

Open Arcnor opened this issue 4 years ago • 4 comments

This is more of a question/discussion, and I haven't found a better place to put it, so please forgive me if this is not the right one.

I'm not sure what C# version is the code targeting, but there are a few things that can be modernized to make the code a bit less verbose (in the places that matter, I understand that verbosity can be a choice on itself), depending again on the version. The ones I was thinking of were:

  • Replacing string.Format with string interpolation
  • Using object initializers for structs
  • Using expression bodied properties (public string myField => _value instead of public string myField { get { return _value; } })
  • Using var instead of explicit types

This is of course subjective, but IMHO it makes the code less verbose in the right places (well, maybe not var, but the rest certainly do)

Let me know what you think, and I'll open a PR if we agree in some/all of them 🙂

Arcnor avatar Aug 12 '19 09:08 Arcnor

The C# code needs to work in any version of Unity, which means it needs to stay compatible with .NET 3.5 (or more precisely, an old version of Mono roughly equivalent to that). We can use newer features if we guard them with appropriate #if's, and provide some other implementation for older environments.

So I'm pretty sure that rules out string interpolation, but I'm not certain about the other features.

As for var, my policy on that is to use it when the resulting type can be read directly from the right-hand side of the assignment. That means I'm all for it with new, or with something like (in Unity) GetComponent<SomeType>. But I'm not OK with it in cases where the type isn't obvious, like var foo = SomeMethodCall();. Just because the compiler can work out the type, doesn't mean I want to work that hard. :)

JoeStrout avatar Aug 12 '19 14:08 JoeStrout

Thanks for your answer!

Well, if Unity versions < 2017.1 are a requirement, then there is not a lot that can be changed, although var and object initializer for structs can be done (both introduced in C# 3), maybe it's worth doing those.

Arcnor avatar Aug 13 '19 04:08 Arcnor

Yes, unfortunately we still have users using Unity 5.5. I'm up for using object initializers for structs where you think that makes the code cleaner. I'm also fine with more use of var as long as it adheres to the policy above.

JoeStrout avatar Aug 13 '19 04:08 JoeStrout

It actually looks like the code is using C#4 features, like default argument values. I'm guessing it's because Mono has a bit of a mix, or because the Mono version on Unity 5.x was 2.8 which had C# 4 enabled by default. In any case, I'll give it a "clean" and open PR, then we can discuss.

Arcnor avatar Aug 13 '19 08:08 Arcnor

Closing this thread now, to help keep the issues list neat. Thank you for all your input!

JoeStrout avatar Oct 31 '23 21:10 JoeStrout