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

Default values are not applied #142

Open
DmitriyBobrovskiy opened this issue Sep 24, 2020 · 3 comments
Open

Default values are not applied #142

DmitriyBobrovskiy opened this issue Sep 24, 2020 · 3 comments
Labels

Comments

@DmitriyBobrovskiy
Copy link

Hi, first of all thank you for this library. It's awesome.

Found out that if I try to set default value to property when there is only getter, then it's not set up.
So here IsCenter will be null

public bool? IsCenter { get; } = true;

But if I make it like this

public bool? IsCenter { get; private set; } = true;

Then it will be true.

@DmitriyBobrovskiy
Copy link
Author

But if I add private set; then I don't see that property in Builder.

@bddckr
Copy link

bddckr commented Sep 26, 2020

I believe this is due to this snippet not actually "copying over" the default value assignments to the builder's properties:

private static IEnumerable<MemberDeclarationSyntax> GetPropertyMembers(RecordDescriptor.Entry entry)
{
return CreateSimpleProperty();
IEnumerable<PropertyDeclarationSyntax> CreateSimpleProperty()
{
yield return
PropertyDeclaration(
entry.TypeSyntax,
entry.Identifier)
.AddModifiers(SyntaxKind.PublicKeyword)
.WithAccessors(
AccessorDeclaration(SyntaxKind.GetAccessorDeclaration).WithSemicolonToken(),
AccessorDeclaration(SyntaxKind.SetAccessorDeclaration).WithSemicolonToken());
}
}

Then once you add private set I think it's not using this for the Record at all as it only accepts read-only properties? I might be wrong on that one.

@amis92
Copy link
Owner

amis92 commented Sep 30, 2020

Hi, yes you're correct - default values aren't copied over to the Builder. Since the record's constructor actually requires passing all of the properties into it, it wouldn't ever be used anyway. It could be indeed if "copied over" into Builder - but it introduced a lot of other headaches (e.g. default value being a class constant/static field would fail to resolve correctly inside Builder).

I won't be developing this project since C#9 provides record types which do exactly everything this project does, but better (and cleaner).

If you'd provide a PR with unit tests, it's possible I'd accept and release it however, because it doesn't seem to be a big change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants