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.

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

ExecuteUpdate: Add SetProperty overload that accepts a value directly (no lambda) #28968

Closed
aradalvand opened this issue Sep 2, 2022 · 7 comments
Assignees
Labels
area-bulkcud closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@aradalvand
Copy link

aradalvand commented Sep 2, 2022

I originally asked this question here but couldn't get a response, so I figured maybe I should create an issue for it. It's about the new ExecuteUpdate feature.

I noticed that the SetProperty method doesn't accept an actual value (a string literal, for example) as its second argument, only an expression, so you'd have to do:

.SetProperty(p => p.Name, _ => "Shay")

Instead of just:

.SetProperty(p => p.Name, "Shay")

Is there a particular reason behind this?! Why don't you just allow simple values?

@aradalvand
Copy link
Author

I would appreciate an answer.

@ajcvickers
Copy link
Member

@aradalvand We try to be responsive to questions asked on GitHub, but that doesn't mean the team can jump onto questions immediately they are asked. The team have many other things to do beyond answering questions here, including having time for a healthy work-life balance.

Other than complicating the API signature and requiring additional code for what seems like limited value, there may also be other reasons why this can't be implemented. We will discuss as a team in due course.

@aradalvand
Copy link
Author

aradalvand commented Sep 5, 2022

@aradalvand We try to be responsive to questions asked on GitHub, but that doesn't mean the team can jump onto questions immediately they are asked. The team have many other things to do beyond answering questions here, including having time for a healthy work-life balance.

I understand that, I wasn't "demanding" an answer or anything, typically issues on the EF Core repo are answered in a day or two in my experience so I thought maybe mine had gotten left out this time.
Thank you for the response.

Other than complicating the API signature and requiring additional code for what seems like limited value, there may also be other reasons why this can't be implemented. We will discuss as a team in due course.

That doesn't make sense.
It doesn't "complicate" the signature, it's just a single overload, and one that you would intuitively expect to be there.

requiring additional code

It requires additional code on your side while eliminating unnecessary code on the user's side; which is a common practice in good API design.

there may also be other reasons why this can't be implemented

Okay, I'd be eager to hear those.

@ajcvickers
Copy link
Member

We discussed this and the consensus was that this would be a good change. We are late in the game for 7.0, so it's more likely to go into 8.0 unless @roji finds some time locked away somewhere.

@aradalvand
Copy link
Author

aradalvand commented Sep 7, 2022

Okay good to hear.
Would be much better if it makes it into 7.0, it's a relatively small thing but it's something that I think almost everyone expects, the API is somewhat incomplete without it.

@zhuangState
Copy link

I'm looking forward to this
.ExecuteUpdate (p => p.Name ="Shay") and .ExecuteUpdate (p =>new { Name ="Shay" ,Age =21 } )

@aradalvand
Copy link
Author

aradalvand commented Sep 10, 2022

@zhuangState I explained why those aren't possible at the moment here.

roji added a commit to roji/efcore that referenced this issue Sep 13, 2022
roji added a commit to roji/efcore that referenced this issue Sep 13, 2022
@roji roji changed the title Why doesn't SetProperty accept a value as its second argument instead of an expression? ExecuteUpdate: Add SetProperty overload that accepts a value directly (no lambda) Sep 18, 2022
@roji roji modified the milestones: Backlog, 7.0.0 Sep 18, 2022
roji added a commit to roji/efcore that referenced this issue Sep 19, 2022
roji added a commit to roji/efcore that referenced this issue Sep 19, 2022
And make SetProperty accept a non-expression lambda directly

Closes dotnet#28968
ajcvickers pushed a commit that referenced this issue Sep 20, 2022
And make SetProperty accept a non-expression lambda directly

Closes #28968
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-rc2 Sep 22, 2022
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 22, 2022
@ajcvickers ajcvickers removed this from the 7.0.0-rc2 milestone Nov 5, 2022
@ajcvickers ajcvickers added this to the 7.0.0 milestone Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-bulkcud closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

No branches or pull requests

4 participants