2
\$\begingroup\$

I have a Sales Order Program that allows each customer to have their own Part Number for one of our Stock Items. The Customer Part Numbers are stored in a separate table. I need to make this as low impact on the User/UI as possible. As a working model I have come up with a related demo using Person for the Order Table and a Table called NickNames that should represent the Customer Part Number Table.

public class Person
{
    private string _sobriquet;

    public Person()
    {
        NickName = new NickName(Id);
    }

    [Key]
    public int Id { get; set; }

    public string FirstName { get; set; }

    public string LastName { get; set; }

    public int NickNameId { get; set; }

    [ForeignKey("NickNameId")]
    public virtual NickName NickName { get; set; }

    public string Sobriquet
    {
        get { return _sobriquet; }
        set
        {
            _sobriquet = value;
            NickName.PersonId = Id;
            NickName.Sobriquet = _sobriquet;
        }
    }
}



public class NickName
{
    public NickName(int personId)
    {
        PersonId = personId;
    }

    protected NickName()
    {

    }

    [Key]
    public int PersonId { get; set; }

    public string Sobriquet { get; set; }
}

This does what I wanted... by Changing the Sobriquet property on the Person table the NickName Table is updated, but I am sure there is a better or cleaner way.

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

As I explain here, you should never initialize reference navigation properties in an entity's constructor. They prevent EF from loading the reference from the database and they impair relationship fixup.

This initialized navigation property ties your Person class to one specific use case: setting Sobriquet. For other cases where loading of the Nickname is required, it has become useless, or crippled at best.

Another downside of your implementation is that you'll never see a Sobriquet until you set it, because _sobriquet will never be set by EF. (I assume that the real property has a [NotMapped] attribute; it can't possibly be mapped).

And then of course there is this golden rule to keep unexpected behavior and side effects away from property setters (and getters, for that matter). By setting Sobriquet, one expects to set Sobriquet only. It is not expected that NickName.PersonId is set too.

Finally, in the two instances where you set NickName.PersonId, i.e. in the property setter and in new NickName(Id), the value of Person.Id can be anything. It's highly unlikely that in every imaginable scenario, Person will have its Id value on time. I'm even pretty confident that in the constructor this will never be the case.

So, as usual, if you want your view to show anything that differs from the model classes, use a view model (or DTO) and map it from/to the model classes, for example using AutoMapper.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.