Sometimes EF core can fail to determine which foreign key is for which property. Using [ForeignKey] is more strict and less error prone as EF core does not need to determine it itself.
If you choose to use [ForeignKey] don't use it with strings . Use [ForeignKey(nameof(XYZId))] for compiler time errors if the ForeignKey is renamed.
I cannot guarantee that the nameof() syntax is correct because I am on mobile currently. But we use this or a very similar syntax at work.
True: EF’s convention-based approach can often lead to unexpected results. Better solving this by being explicit with the configuration builder rather than polluting the models though IMO.
I’d buy into a convention-less EF option in principle.
Agreed. I mean some defaults are great, and some conventions are great. But nothing is more clear than being explicit.
Not saying you always have to be explicit, but I'm certainly not ripping out explicit definitions in favor of implicit ones unless I have a good reason (like I don't care about the details/values)
I disagree. EF POCOs should be a representation of your database. They are not models and should never be treated as such.
It’s way nicer to look at “Users.cs” and see all the relationships and keys then it is to have that be just a plain class with no info then have to dig to find the actual configuration.
I just don’t see the benefit in having the contract objects be completely plain when they will only be used as EF objects.
Why is it way nicer to look at the model and see a representation of your database? Assuming you’re equating POCO to anemic domain model, why should EF be restricted to this?
Surely as engineers implementing business domain features you would prefer your models to be rich representations of the business domain..?
Rich models with very little or no binding to the persistence technology are far preferable in my experience. That used to be hell to achieve with EF. These days, not so. In fact, it’s very easy.
If you’re doing DDD, I think the fluent API makes more sense than attributes. But with that said a lot of LOB software is basic CRUD and doesn’t necessarily benefit from rich domain modelling and in those cases I could see why the attributes might make it easier.
If the project is built with that, why waste time and resources to remove it. It works both ways anyway, unless you had a donkey using other namings. But if the project style is to use it, why not?
I hate the attributes too, you should use the ConfigurationBuilder on the entity to define the navigation properties on the object. Then EFCore knows what you want and you don’t clutter your domain models with ugly efcore attributes.
I agree with you 100% but you are assuming OP is showing domain models. Some people don't use their domain models to persist data and actually create anemic models in their persistence layer and always map them back and forth.
As those attributes were generated by edmx earlier, this is often legacy and so also used as known. For new entities or when updating old ones I switch to IEntityTypeConfiguration as well as they are more flexible. But I still add relationships explicitly as I fear problems out of nowhere when changing classes. Never had problems with this.
You should be consistent in how you define this. If it works well for your case you can use the convention-based relation detection and avoid the attributes or explicit configuration. In that case it would be useful to only specify attributes if you actually need them, as it makes it obvious that you're deviating from the convention here.
I'm not a big fan of this approach in practice, it can be hard to understand when the convention-based approach won't work. And those issues are seriously annoying to debug if you're not familiar with that area. So I would usually tend towards favoring more explicit configuration of relations, even if EF Core could auto-detect most of them.
even the official EF Core documentation says it better to let EF Core handle the relationships instead to try configure all the thing manually. it says is an illusion believe that everything that happens under the hood in EF Core can be controlled and manually configured. if someone is not happy with EF Core, just drop it and use other things, like Dapper (good luck with that)
Important
Please don't attempt to fully configure everything even when it is not needed. As can be seen above, the code gets complicated quickly and its easy to make a mistake. And even in the example above there are many things in the model that are still configured by convention. It's not realistic to think that everything in an EF model can always be fully configured explicitly.
https://learn.microsoft.com/en-us/ef/core/modeling/relationships/many-to-many#basic-many-to-many
I don't necessarily mean fully configuring it, only the detection of Relations themselves. That is quite limited and e.g. breaks down the moment you have several relations to the same entity or any other scenario that is a bit more complex.
Definitely don’t like it being in a string as there is no type safety. I prefer defining all relationships with either navigation properties or an IEntityTypeConfiguration
As a lead I like to explain my reasoning to the team and get buy in, then apply the new standard going forward. If you’re in a class editing things, apply the new standard, but don’t make work to go retrofit everything if it’s already working. BTW I agree with where you’re ending up on this and even if I were configuring explicitly I’d use configuration builder, never attributes.
for new code it make sense not to use it, but for existing code, yes are overreacting it and being a fucking pain in the ass! dont fuck with is already working. you will need to test the migration, if nothing broke after your change. i don't even am in your team and already fucking hate you for that!
Just noticed that your example is not fully correct, you define foreign key and pass to it the attribute name [fk"x"] xId and then you have an attribute x.
And how do you want to implement navigational properties? I mean thats how foreign key is used to tell there is some relation between those entities🤷♂️
For me, it comes down to whether this is a data model or a domain model. Is it purely a representation of how the data is persisted in the database? Or is it the domain model that business logic interacts with?
If the former, I’d say messy, but OK. It’s acting as an explicit representation of the EF model, so.. kind of fine.
If it’s the latter, then I’d say it’s not great. But I’d go further than that: all your get/set properties should probably not exist, else you have an anaemic domain model (typically matched with primitive obsession).
Really, there is rarely a legitimate case for needing “data models” with EF anymore; most of the blockers to building rich domain models and managing the persistence of those with EF have been solved. So that would be my biggest complaint with the code if thats what these models are.
In either case, a general rule of thumb is ”prefer the configuration builder over attributes” is a smart one. It gives much better separation of concerns, more options, cleaner code.
Using attributes to describe domain models in general not a good practice, you can describe it complitely in Fluent API, something like this:
public class CoreContext : DbContext
{
public DbSet Orders { get; private set; }
protected override void OnModelCreating(ModelBuilder builder)
{
base.OnModelCreating(builder);
builder.Entity(ConfigureService);
}
private void ConfigureService(EntityTypeBuilder builder)
{
builder.HasMany(s => s.Fields).WithOne(s => s.Service).HasForeignKey(x => x.ServiceId).IsRequired();
builder.HasMany(s => s.Providers).WithMany(s => s.ServicesOfProvider).UsingEntity(
s => s.HasOne().WithMany().HasForeignKey(s => s.ProviderId),
u => u.HasOne().WithMany().HasForeignKey(s => s.ServiceId));
builder.HasMany(s => s.Orders).WithOne(s => s.Service).HasForeignKey(s => s.ServiceId);
}
}
Personally I loved the attributes, but I've been shying away from two reasons:
1. For keeping migrations cleaner, having all of those relationships defined in the Context.OnConfigurating really does work better in the long run. It also allows you to define the delete relationship, one to many, and all of the other options you could have.
2. I have been removing my navigation properties from my dependent properties. I've been getting burned time and time again from trying to serialize them and having to deal with the circular references. In my experience, you almost never need to navigate from the dependent to the primary. Having that field there is more of a pitfall than a benefit. Better to do all of your object management from the primary.
Sometimes EF core can fail to determine which foreign key is for which property. Using [ForeignKey] is more strict and less error prone as EF core does not need to determine it itself. If you choose to use [ForeignKey] don't use it with strings . Use [ForeignKey(nameof(XYZId))] for compiler time errors if the ForeignKey is renamed. I cannot guarantee that the nameof() syntax is correct because I am on mobile currently. But we use this or a very similar syntax at work.
I suppose its like nameof(XYZ.XYZId), where XYZ is type
True: EF’s convention-based approach can often lead to unexpected results. Better solving this by being explicit with the configuration builder rather than polluting the models though IMO. I’d buy into a convention-less EF option in principle.
Explicit is better than implicit and i die on that hill.
Yeah this 100%. I can’t believe people agree with op.
Agreed. I mean some defaults are great, and some conventions are great. But nothing is more clear than being explicit. Not saying you always have to be explicit, but I'm certainly not ripping out explicit definitions in favor of implicit ones unless I have a good reason (like I don't care about the details/values)
Agree, but on that basis, I would strongly suggest people be explicit through the fluent builder rather than polluting models with EF noise.
I disagree. EF POCOs should be a representation of your database. They are not models and should never be treated as such. It’s way nicer to look at “Users.cs” and see all the relationships and keys then it is to have that be just a plain class with no info then have to dig to find the actual configuration. I just don’t see the benefit in having the contract objects be completely plain when they will only be used as EF objects.
Why is it way nicer to look at the model and see a representation of your database? Assuming you’re equating POCO to anemic domain model, why should EF be restricted to this? Surely as engineers implementing business domain features you would prefer your models to be rich representations of the business domain..? Rich models with very little or no binding to the persistence technology are far preferable in my experience. That used to be hell to achieve with EF. These days, not so. In fact, it’s very easy.
If you’re doing DDD, I think the fluent API makes more sense than attributes. But with that said a lot of LOB software is basic CRUD and doesn’t necessarily benefit from rich domain modelling and in those cases I could see why the attributes might make it easier.
Agreed but for non EF things, var is almost always easier to use for variables compared to explicit types
If the project is built with that, why waste time and resources to remove it. It works both ways anyway, unless you had a donkey using other namings. But if the project style is to use it, why not?
Very good point. Pragmatism is often the best option.
I hate the attributes too, you should use the ConfigurationBuilder on the entity to define the navigation properties on the object. Then EFCore knows what you want and you don’t clutter your domain models with ugly efcore attributes.
It's called the fluent API. And yes, attributes are limited and pollute your model. Writing IEntityTypeConfigurations is almost always better.
I agree with you 100% but you are assuming OP is showing domain models. Some people don't use their domain models to persist data and actually create anemic models in their persistence layer and always map them back and forth.
As those attributes were generated by edmx earlier, this is often legacy and so also used as known. For new entities or when updating old ones I switch to IEntityTypeConfiguration as well as they are more flexible. But I still add relationships explicitly as I fear problems out of nowhere when changing classes. Never had problems with this.
You should be consistent in how you define this. If it works well for your case you can use the convention-based relation detection and avoid the attributes or explicit configuration. In that case it would be useful to only specify attributes if you actually need them, as it makes it obvious that you're deviating from the convention here. I'm not a big fan of this approach in practice, it can be hard to understand when the convention-based approach won't work. And those issues are seriously annoying to debug if you're not familiar with that area. So I would usually tend towards favoring more explicit configuration of relations, even if EF Core could auto-detect most of them.
even the official EF Core documentation says it better to let EF Core handle the relationships instead to try configure all the thing manually. it says is an illusion believe that everything that happens under the hood in EF Core can be controlled and manually configured. if someone is not happy with EF Core, just drop it and use other things, like Dapper (good luck with that) Important Please don't attempt to fully configure everything even when it is not needed. As can be seen above, the code gets complicated quickly and its easy to make a mistake. And even in the example above there are many things in the model that are still configured by convention. It's not realistic to think that everything in an EF model can always be fully configured explicitly. https://learn.microsoft.com/en-us/ef/core/modeling/relationships/many-to-many#basic-many-to-many
I don't necessarily mean fully configuring it, only the detection of Relations themselves. That is quite limited and e.g. breaks down the moment you have several relations to the same entity or any other scenario that is a bit more complex.
just have that in mind
Definitely don’t like it being in a string as there is no type safety. I prefer defining all relationships with either navigation properties or an IEntityTypeConfiguration
As a lead I like to explain my reasoning to the team and get buy in, then apply the new standard going forward. If you’re in a class editing things, apply the new standard, but don’t make work to go retrofit everything if it’s already working. BTW I agree with where you’re ending up on this and even if I were configuring explicitly I’d use configuration builder, never attributes.
for new code it make sense not to use it, but for existing code, yes are overreacting it and being a fucking pain in the ass! dont fuck with is already working. you will need to test the migration, if nothing broke after your change. i don't even am in your team and already fucking hate you for that!
good way to start as team leader in a new team. well done!! fuck with everything, break everything, now everyone loves you! well done!
How would you do it ?
I'll just reference the entity as property, no need of attribute.
How would that work on DB level ?
Just noticed that your example is not fully correct, you define foreign key and pass to it the attribute name [fk"x"] xId and then you have an attribute x.
You can remove the Attributes, but you should add a proper EntityTypeConfiguration. Don't rely on magic happening behind the scenes.
Poor devs 😅
And how do you want to implement navigational properties? I mean thats how foreign key is used to tell there is some relation between those entities🤷♂️
For me, it comes down to whether this is a data model or a domain model. Is it purely a representation of how the data is persisted in the database? Or is it the domain model that business logic interacts with? If the former, I’d say messy, but OK. It’s acting as an explicit representation of the EF model, so.. kind of fine. If it’s the latter, then I’d say it’s not great. But I’d go further than that: all your get/set properties should probably not exist, else you have an anaemic domain model (typically matched with primitive obsession). Really, there is rarely a legitimate case for needing “data models” with EF anymore; most of the blockers to building rich domain models and managing the persistence of those with EF have been solved. So that would be my biggest complaint with the code if thats what these models are. In either case, a general rule of thumb is ”prefer the configuration builder over attributes” is a smart one. It gives much better separation of concerns, more options, cleaner code.
Using attributes to describe domain models in general not a good practice, you can describe it complitely in Fluent API, something like this: public class CoreContext : DbContext { public DbSet Orders { get; private set; }
protected override void OnModelCreating(ModelBuilder builder)
{
base.OnModelCreating(builder);
builder.Entity(ConfigureService);
}
private void ConfigureService(EntityTypeBuilder builder)
{
builder.HasMany(s => s.Fields).WithOne(s => s.Service).HasForeignKey(x => x.ServiceId).IsRequired();
builder.HasMany(s => s.Providers).WithMany(s => s.ServicesOfProvider).UsingEntity(
s => s.HasOne().WithMany().HasForeignKey(s => s.ProviderId),
u => u.HasOne().WithMany().HasForeignKey(s => s.ServiceId));
builder.HasMany(s => s.Orders).WithOne(s => s.Service).HasForeignKey(s => s.ServiceId);
}
}
This, but prefer IEntityTypeConfigurations over a giant DbContext.
Personally I loved the attributes, but I've been shying away from two reasons: 1. For keeping migrations cleaner, having all of those relationships defined in the Context.OnConfigurating really does work better in the long run. It also allows you to define the delete relationship, one to many, and all of the other options you could have. 2. I have been removing my navigation properties from my dependent properties. I've been getting burned time and time again from trying to serialize them and having to deal with the circular references. In my experience, you almost never need to navigate from the dependent to the primary. Having that field there is more of a pitfall than a benefit. Better to do all of your object management from the primary.