Skip to content

Add support of Interpolated strings.#530

Merged
StefH merged 3 commits intozzzprojects:masterfrom
yangzhongke:main
Jul 17, 2021
Merged

Add support of Interpolated strings.#530
StefH merged 3 commits intozzzprojects:masterfrom
yangzhongke:main

Conversation

@yangzhongke
Copy link
Copy Markdown
Contributor

Add support of Interpolated strings.
see #527

I have added two test cases: QueryableTests.FormattableString.cs and EntitiesTests.FormattableString.cs. All the tests are passed:
image

@StefH StefH changed the title Add support of Interpolated strings. https://github.com/zzzprojects/S… Add support of Interpolated strings. Jul 1, 2021
@StefH
Copy link
Copy Markdown
Collaborator

StefH commented Jul 1, 2021

Hello @yangzhongke, thank you for your contribution.

However, I thought that this functionality would be integrated in the current code, and not creating new extension methods.

Can you change your PR?

@yangzhongke
Copy link
Copy Markdown
Contributor Author

yangzhongke commented Jul 2, 2021

If there are two overloaded methods as below:

  1. Where(string predicate,params object[] args)
  2. Where(FormattableString predicate)

Due to the deliberate decision by the Microsoft , The code Where($"Id={id}") will invoke Where(string predicate,params object[] args) instead of Where(FormattableString predicate), which lead to my decision to create new method names, like WhereInterpolated instead of using Where.

@StefH
Copy link
Copy Markdown
Collaborator

StefH commented Jul 2, 2021

Can't you just add a new method like?

Existing...

public static IQueryable Where([NotNull] this IQueryable source, [NotNull] ParsingConfig config, [NotNull] string predicate, params object[] args)

New...

public static IQueryable Where([NotNull] this IQueryable source, [NotNull] ParsingConfig config, [NotNull] FormattableString predicate)

@yangzhongke
Copy link
Copy Markdown
Contributor Author

yangzhongke commented Jul 3, 2021

I can. But where($" id={id}") will invoke the "string predicate" version instead of "FormattableString" version.
So I have to create the WhereInteroplated(FormattableString fs) method to match WhereInteroplated($" id={id}").
I have mentioned the reason before.
Make sense?

@yangzhongke
Copy link
Copy Markdown
Contributor Author

Even EF Core created two methods to execute sql statements, ExecuteSqlRaw(string version) and ExecuteSqlInterpolated(FormatableString version). So I reckon that the two different methods solution, Where and WhereInterpolated , is a tradeoff.

@yangzhongke
Copy link
Copy Markdown
Contributor Author

Agree with the renaming.
Should I do something to do the changes?

@StefH
Copy link
Copy Markdown
Collaborator

StefH commented Jul 8, 2021

@yangzhongke
Yes, if possible, change your PR code.

…rmattableStringExtensions.cs and DynamicQueryable_FormattableString_Extensions.cs --> DynamicQueryableWithFormattableStringExtensions.cs
@yangzhongke
Copy link
Copy Markdown
Contributor Author

I have renamed those two files and committed the changes.
There are two files: DynamicQueryableWithFormattableStringExtensionsand EFDynamicQueryableWithFormattableStringExtensions.
DynamicQueryableWithFormattableStringExtensions.cs contains: AllInterpolated and other XXXInterpolated methods, and EFDynamicQueryableWithFormattableStringExtensions contains: AllInterpolatedAsync and other XXXInterpolatedAsync methods.
DynamicQueryableWithFormattableStringExtensionsand .cs is already on the src/System.Linq.Dynamic.Core folder; because EFDynamicQueryableWithFormattableStringExtensions uses Microsoft.EntityFrameworkCore, just like EntityFrameworkDynamicQueryableExtensions.cs, so EFDynamicQueryableWithFormattableStringExtensions should be on the folder Microsoft.EntityFrameworkCore.DynamicLinq.EFCore3.

Comment thread src/System.Linq.Dynamic.Core/DynamicQueryableWithFormattableStringExtensions.cs Outdated
@StefH StefH merged commit 4008215 into zzzprojects:master Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants