Skip to content

Conversation

Romfos
Copy link
Contributor

@Romfos Romfos commented May 14, 2024

Changes:

  • Use c# 12 primary constructors

@Romfos Romfos marked this pull request as ready for review May 14, 2024 12:15
@Romfos
Copy link
Contributor Author

Romfos commented May 14, 2024

One more discussion point:

I can add editorconfig option in next pr (because maybe you want to merge another PR and I don't want to block them now) for this and VS will warn to use primary constructors if possible.
It will make code style more consistent.
Do we want to have it or we need to have a more flexibility at this area?

Ready to hear you opinions

@dtchepak dtchepak merged commit b7ff02c into nsubstitute:main May 20, 2024
@dtchepak
Copy link
Member

Thanks @Romfos . I think it's fine to add editorconfig.

@zvirja
Copy link
Contributor

zvirja commented Mar 10, 2025

@Romfos Just out of curiosity, do you find this code more readable? I was considering using primary constructors in my personal projects, but gave up every single time, as it's just makes the code messy. Consider the following example:

public class Record(string name)
{
    public void DoSomething(string action)
    {
        // At this point it's totally unclear what `name` is - local variable, method argument or class field.
        // With _name it was obvious, but not with `name` - it has a naming convention of local variable.
        var xx = name;
    }
}

IMO distinction between object state and non-object state properties is crucial when you design code, as you often treat them differently and have different considerations e.g. in terms of thread-safety.

I never found primary constructors practical. Maybe if you only use it for fields initialization - sure... But that's not the case, as they are accessible in the whole file.

What is your experience? Do you find code indeed more better after the change? Yeah, it's more compact - but is it more readable? What am I missing with the modern trends? 🤔

@Romfos @dtchepak

@Romfos
Copy link
Contributor Author

Romfos commented Mar 10, 2025

Hello,

@zvirja Thank you for feedback.

From my point of view this is mostly a "developer preference" then a real decision, but Idea is simple:

In 99% cases you don't do any real work in constructors, you just assign args to fields\properties.

For "classic syntax" you need:

  1. Create proeptties\fields
  2. Declare the same ctor with parameters
  3. Assign them correctly

Benefits of new syntax:

  1. More compact
  2. You just have a "less space" to make a typo or miss assignment or any other stupid mistake
  3. More stable to refactroings. rename in 1 place automatically rename in another

at the same time I agree that resharper or any similar tool also help to mitigate these problems

About private fields:
For me is unclear - what is the benefit to know is it a private fields or not?
Sometime I'm using "go to definition" feature in VS if need to read a declaration but for me personally no value to have it on "syntax level"
For me this is current standard on dotnet project since release of this feature

We can easy revert it at any moment of time, this is just an editorconfig option, if you think that classic syntax makes more value. This is not too late!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants