Performance loss on using united id?
if some entity was using AspNet User (string Id's) what I did was instinctly doing
and pass that to every entity i see
15 Replies
If you're going to use this interface instead of the concrete type of the entity in some of your code, or use a constrained generic, it's going to compile to an uninlineable interface method call (unless the whole outer method was inlined for your concrete type), which is at best 20 times slower than a field access (which a regular property access will certainly be inlined to for a concrete type)
So yes, it would be slower if you used that interface directly or as a constraint
When used in a Linq expression, it's going to be one extra cast node to IEntity<string> when you access the id
which is always going to be a bit more memory and a bit more work to process it out
So it will always be slower performance-wise to use it than to use a concrete type
The main concern of modern apps however is usually maintainability over performance
So if that interface makes it easier for you to reuse code, it's worth from this standpoint, even if it makes your whole app slower
It's not as much of a perf difference of using or not using the interface vs using or not using EF Core
EF Core does an insane amount of work with the query compilation and the entity tracking and other stuff
And that uses lots of reflection and indirection itself
By the way, using an interface for the collection types is even worse than this
Or using linq in any way
so if you don't feel uneasy about those, stop worrying over your id interface
@spit on that Thang CHO BOC
Btw, if you used a base class rather than an interface, none of this would apply
Not saying you should, just that it is going to be the same perf as with a concrete class
Oh okay, since i never understood it on why the interface was needed from my uni lecture
I kinda copy pastad it
So I can live without the Ientity and Irepository?
And also I wanted to emulate realworld Web apps
In the DAL I don't use Automapper Mapperly just basic manual code since I just doing new entity from existing entity
In BL however I should do Mapperly right?
I worked with that in my last project for a table that has FK from 2 other tables the generated query was over 92 lines of code
I don't know how it works properly, what would you recommend?
what I had going on was
what is this for?
you need them if you use them
Actually I see no references i never used them....
Shit
well if you don't use it then it just sits there as metadata
if you don't actually use it, it doesn't affect the perf or anything about the program
if I send the EF logic as zip can you check for anything really stupid i would like to learn from the mistakes
no thanks
put it on github
GitHub
GitHub - ThangChoBoc/DALandBL
Contribute to ThangChoBoc/DALandBL development by creating an account on GitHub.
the other things like API and That i skipped because its not the main concern yet
i wanted to make sure that the EF implementation is as good as it can get
I asked copilot
so we use Mapper as a setting for the Automapper / Mapperly to get what we want?
I have no idea what this means
also, copilot is dumb
don't take it seriously
if you think you'd benefit from a mapper, use a mapper
if not — map manually
if you're asking at what point you should be mapping, then you either don't understand your abstraction yet, so write more code and figure it out, or there is a pointless abstraction in the way that is making you consider unimportant things
I will look further in this, thank you for the help 🙏
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/ApplicationDbFactory.cs#L14-L17
Path.GetFullPath(@"..\ZelnyTrh");
does the same.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/UnitsOfWork/UnitOfWork.cs#L7
The general recommendation is to use EF Core directly as your unit of work + repositories.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Seeders/RoleSeeder.cs#L42-L46
I mean, nothing bad about it besides having the password exposed. How are you planning to fix this?
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Seeders/Seeders.cs#L84-L87
That's already the default? Why do you need that?
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Entities/CropCategories.cs#L21-L27
virtual
is bad here. Also, i recommend sealing every class you don't plan to inherit. Especially entities.
You don't need it to be ICollection
either. People generally make it a list. And it is generally recommended to not default initialize it (because you're likely not using it). Set it to null!
. But I mean it is a controversial thing and it's fine to have it initialized, you'd just be creating objects for no reason (probably).
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Entities/Offers.cs#L9
You don't need this.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Entities/Offers.cs#L22
Or this.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Entities/Offers.cs#L28
Or this.
Those are the defaults already. Look up "EF Core conventions".
Wait, why are your entities named in plural?
https://github.com/ThangChoBoc/DALandBL/tree/main/DAL/Mappers
This is a pointless abstraction probably. Just make extension methods if you're doing this.
If you're gonna be migrating to a library for this, it's more deserved
But as is, it's just pointless
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Repositories/Repository.cs#L6
Yeah, like I said, generally you just use EF Core. What you did just limits what you get to use from EF Core intentionally. It's pointless.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/BL/Mappers/UserMappingProfile.cs#L16
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/BL/Mappers/UserMappingProfile.cs#L18
It is bad to have business logic in your mappers. Mappers are for mapping.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/BL/Mappers/UserMappingProfile.cs#L56-L73
This is plain dumb imo. There's a method to opt in what you map iirc
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/BL/Mappers/SelfPickupRegistrationMappingProfile.cs#L17
Look! Business logic again. Really bad.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/BL/Services/AttributeService/AttributeDefinitionService.cs#L11
CancellationToken?
Never use async overloads that don't accept a cancellation token.
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/BL/Services/AttributeService/AttributeDefinitionService.cs#L89
Pointless comments
I guess it's just the conventional clean architecture. Not how I'd do it personally, I try to go more specific and jam as much as I can in a single query, so there are less round-trips.
You've got an insane amount of round trips (every method call into a repository is a round-trip), but I guess that's just what people do with EF Core
I think technically your project is just by the book
I recommend you learn SQL
You might be misunderstanding the role of a mapper. Mappers are supposed to automate mapping partially. So it figures out what to map to what automatically (as much as possible).the thing is i can do basics but i didnt how to use sql strings code might have to look into it if I dont want to seach one table with generated 70+ LOC SQL calls to get the data I wanted like in the Github section i uploaded. Thank you for the insight, and yes I am following this cookbook from my uni course(it's for an standalone app but still using it for a web app)
https://github.com/nesfit/ICS/tree/master/src/CookBook
The seeders was a requirement for the course they wanted us to send the sln for them to run the database from scratch and when ran it should not be empty. In my case now with the new project I don't have this concern although I feel like if I want to sell them something they should be able to run locally without the seeders tbh. (maybe seeding for the items like producs)
Essentially the new project is a web app that will behave like this site which will be my main competitor if I manage to sucessfully make it work flawlessly (gokasa.cz)
thank you for the feedback! I have learned a lot from this interaction
this segment:
https://github.com/ThangChoBoc/DALandBL/blob/719f553617236e46f98e10093b35cf7cbc6412c7/DAL/Seeders/Seeders.cs#L84-L87
That's already the default? Why do you need that?
i think i put everything required so that might be case here (i forgot)
I meant learn sql to understand the generated queries
ah yeah you did add required to collections
you don't need that
since you have default values
it doesn't make sense to have both