2

so as an example lets say we have the following situation: I have a class called StandardEngineeredModel which has properties like ModelNumber, VoltageInput, VoltageOutput etc.

I also have a class called Fuse that has properties like Designator, Rating, and Type. In my database model these two classes would have a many to many relationship with eachother, a StandardEngineeredModel can contain MANY fuses, and a fuse can be contained in MANY different StandardEngineeredModels. See code below.

public class StandardEngineeredModel
{
    StandardEngineeredModel()
    {
        Fuses = new List<Fuse>();
    }

    [Key]
    public string ModelNumber { get; set; }
    public int VoltageInput { get; set; }
    public string VoltageOutput {get;set;}

    public ICollection<Fuse> Fuses { get; set; }
}

public class Fuse
{
    [Key,Column(Order = 0)]
    public string Designator { get; set; }
    [Key, Column(Order = 1)]
    public string Rating { get; set; }
    [Key, Column(Order = 2)]
    public string Type { get; set; }

    public ICollection<StandardEngineeredModel> StandardEngineeredModels { get; set; }
}

So my question is the portion where you initialize the collection in the constructor: For my StandardEngineeredModel it makes sense to initialize the Fuses with a list. But in my Fuse class it doesn't really make sense for me to initialize the ICollection because typically I will assign fuses to a StandardEngineeredModel not assign a StandardEngineeredModel to a fuse.

Is there any issue with doing this that I am not seeing? What types of problems can this cause for me down the road? I have used EF in a few of my applications but have yet to need a Many to Many relationship so I am looking for some general advice over this.

Thanks in advance!

Eric Obermuller
  • 245
  • 3
  • 19
  • "But in my Fuse class it doesn't really make sense for me to initialize the ICollection because typically I will assign fuses to a StandardEngineeredModel not assign a StandardEngineeredModel to a fuse." I think you answered your own question. You probably don't want to model fuses that know something about all devices that use them. But the converse does make sense - it makes sense for devices to know what fuses they use. – chadnt May 18 '18 at 20:17
  • Yeah the only reason I was asking is because in most tutorials for Many to Many relationships they say to initialize both sides of the relationship. I just wanted to make sure what I was thinking made sense haha. Thanks! @chadnt – Eric Obermuller May 18 '18 at 20:22
  • Assuming that this is EF6 (since you don't mention an explicit junction class as EF-core would require): the collection `StandardEngineeredModels` [isn't even necessary](https://stackoverflow.com/a/8136709/861716). – Gert Arnold May 18 '18 at 21:00

2 Answers2

-1

You don't technically have to initialize the collections in the constructor, but you need to be aware that they will be null (especially since you are not using the virtual keyword that enables lazy loading) and you need to check for that throughout your app - otherwise, you could get null exceptions in unexpected places.

Ben
  • 1,032
  • 8
  • 18
  • Maybe the OP does not want lazy loading. The 2 cents offered in this answer should have been just a comment. -1 – CodingYoshi May 18 '18 at 20:36
  • the key takeaway from this post was that the OP needs to be mindful of the fact that the collection will be null - I don't know why you read that and only took away the bit about lazy loading? – Ben May 18 '18 at 21:00
-1

But in my Fuse class it doesn't really make sense for me to initialize the ICollection because typically I will assign fuses to a StandardEngineeredModel not assign a StandardEngineeredModel to a fuse.

What you say above is true but, I think, you are confusing these Entity Models with a Domain Model. In your domain model you may have a method in your StandardEngineeredModel:

public void AssignFuses(List<Fuse> fuses)

But in your Fuse, you will not have a method like this:

public void AssignStandardEngineeredModels(
    List<StandardEngineeredModel> sems) // makes no sense

However, your domain model clients (by clients I mean other classes, or devs) may have the need to navigate from a Fuse to the StandardEngineeredModel that they are assigned to so how will they do it? Well the Fuse class will need a readonly List<StandardEngineeredModel> to aid with this.

Ok now back to your Entity Model:

Is there any issue with doing this that I am not seeing? What types of problems can this cause for me down the road?

As a good practice if you have a class with other classes in it, it is nice to initialize them. Why? Because we want to be nice to ourselves and other devs. Consider this:

public class A
{
    public List<string> SomeThings{ get; set; }
    public A(){}
}

I am using your class, I cannot see the code because I am using the DLL. I do this:

var a = new A();
a.SomeThings.Add("1");

I compile, I give the code to QA and they forget to test this very specific case and it goes to production and BAM!!!

So follow good practices and initialize it. I am not sure if EF will fail if you do not have it initialized so perhaps you can test and figure that out.

One Suggestion

I would suggest not naming your entity models with the suffix Model. If my guest is correct, you are probably doing this for MVC and thinking this is your model. But the issue is that you are designing your table names with the intention it will be used in MVC. What if it is not? Also, there will be cases when this model is not sufficient enough to be a model in MVC and may require additional properties.

CodingYoshi
  • 25,467
  • 4
  • 62
  • 64
  • two things: 1) the main issue here is that it will be null and that will come back to bite him (also, no lazy loading), and 2) why do you post an answer saying `I am not sure if EF will fail if you do not have it initialized so perhaps you can test and figure that out.` - you should know whether this works or not if you are able to answer the question. This answer is misleading. – Ben May 18 '18 at 20:30
  • @Ben 1) Yes it could bite him and I mentioned that. So what if no lazy loading? 2) Because I gave many other reasons and I am not sure about EF. His question is not only about EF. 3) It is not misleading at all. How am I misleading him/her? – CodingYoshi May 18 '18 at 20:35
  • You start your answer with some imaginary method `AssignFuses` which was not part of the original question and is also not how you would create a many to many relationship by conventional practice. Then, after giving advice which is bad practice, you launch in to a discussion about how important good practice is. Finally, you give an example that would throw a null reference exception but you don't explain that anywhere in the post - instead you talk about poor QA and bugs getting in production. The answer is wordy and filled with irrelevant and misleading content. – Ben May 18 '18 at 20:58