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

Adding new parent class example in README #183

Merged
merged 2 commits into from
Feb 2, 2022

Conversation

pdevito3
Copy link
Contributor

This adds a new example to the docs as discussed in #162. Adapted it to match the example in there already. Let me know if you want any mods!

@ardalis ardalis merged commit 1e09282 into ardalis:main Feb 2, 2022
@pdevito3 pdevito3 deleted the docs-update branch February 22, 2022 13:55
@pyce-lb
Copy link

pyce-lb commented Oct 15, 2022

I hope this finds you even though the PR is already merged!

Whilst I was trying to understand the benefits of SmartEnum I was struggling to understand this example. I was questioning my knowledge of nested classes and their visibility, but I think I figured out what confused me.

The code added in this PR does not compile.
I presume it was meant to look like the code below (and there was some residue left after copy-pasting code from #162), although I still can't fathom what it really tries to demonstrate.

public class Manager
{
    private EmployeeType _employeeType { get; set; }

    public Manager(EmployeeType employeeType) => _employeeType = employeeType;

    public string Type
    {
        get => _employeeType.Name;
        set
        {
            if (!EmployeeType.TryFromName(value, true, out var parsed))
            {
                throw new Exception($"Invalid manage type of '{value}'");
            }
            _employeeType = parsed;
        }
    }

    public decimal BonusSize => _employeeType.BonusSize;
}

P.S. I would also suggest a different phrase instead of "parent class" as it is usually associated with inheritance, but in this case Manager is not derived from anything, it just holds a reference to EmployeeType.

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