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.

Sign up for GitHub

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

Jump to bottom

Use of Context.SaveChanges() in Create() method of EF Repository #10

Closed
jersiovic opened this issue Jan 30, 2015 · 5 comments
Closed

Use of Context.SaveChanges() in Create() method of EF Repository #10

jersiovic opened this issue Jan 30, 2015 · 5 comments
Labels
enhancement

Comments

@jersiovic
Copy link
Contributor

Related to Create(T entity) method within EF repository to what extend is useful to use Context.SaveChanges() within Create method?
I mean, I understand you use it for having available for next queries the entitity you just added. But it is an expesive price for having this commodity. In your opinion is this commodity really a need?
I mean in Orchard 1.8 the current code that consumes Repository really harness this commodity?
How much code? Can you point me an example? Will it be difficult to change that code that adds and consumes data to delegate on it the decission of when to call context.SaveChanges()?
@Jetski5822
Copy link
Member

Yes it is an expensive commodity - it's one of the things I really dislike about EF - there is a lot I like, but this one area does bug me. I think it should be added in by settings, like "AutoFlush" : true within a config file.

@jersiovic
Copy link
Contributor Author

What exactly you mean with the Autoflush feature? What do you expect it does? When would it save data? With this autoflush features you would expect that when you query the context any created data but not saved in db would be returned because if it still is not saved previously EF will save it? Is this what you mean?

Enviado desde mi Windows Phone

De: Nicholas Maynemailto:[email protected]
Enviado: ‎31/‎01/‎2015 11:31
Para: OrchardCMS/Brochardmailto:[email protected]
CC: jersiovicmailto:[email protected]
Asunto: Re: [Brochard] Use of Context.SaveChanges() in Create() method of EF Repository (#10)

Yes it is an expensive commodity - it's one of the things I really dislike about EF - there is a lot I like, but this one area does bug me. I think it should be added in by settings, like "AutoFlush" : true within a config file.

Reply to this email directly or view it on GitHub:
#10 (comment)

@Jetski5822
Copy link
Member

The idea of AutoFlush would be that if you add/update an object, a call to SaveChanges is made.

Or alternatively, if you add/update an object we set a isdirty flag, then when making a query that involves that object, we call save changes and then the query will return everything.

Not sure...

@jersiovic
Copy link
Contributor Author

I see, I think both alternatives are expensive.
The point is why do you need that? Do you have many code where you need to mix in memory data pending of being added with data in the db? Can you point me code in Orchard that has this need. I don't know too many of Orchard but I suspect we are worried for something we don't have any need.

@Jetski5822 Jetski5822 added this to the Data layer milestone Feb 2, 2015
@Jetski5822
Copy link
Member

Removed Repository pattern.

Skrypt pushed a commit that referenced this issue Jan 25, 2024
@hishamco @Skrypt
^ This is a combination of 11 commits.
5df265b 
^ This is the 1st commit message:

Add GetCulture() extension method

^ This is the commit message #2:

Cleanup ISmsService (#15142)


^ This is the commit message #3:

Fix TheAdminTheme layout margin and padding (#15143)


^ This is the commit message #4:

Fix SectionDisplayDriver prefix (#15123)


^ This is the commit message #5:

Prefill template name when creating a template. (#15145)


^ This is the commit message #6:

Set the User Localization feature priority

^ This is the commit message #7:

Fix issue with default culture not selected

When currentUserCulture is null or supportedCulture doesn't contain currentUserCulture.

^ This is the commit message #8:

Update the height of the admin content (#15153)


^ This is the commit message #9:

Eliminate the anti-discovery pattern in Elasticsearch (#15134)


^ This is the commit message #10:

Renaming and cleaning up search services (#15156)


^ This is the commit message #11:

mkdocs-material 9.5.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Jetski5822 @jersiovic