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

Query/MQ: consider making ExpressionPrinter static #29851

Closed
maumar opened this issue Dec 14, 2022 · 1 comment · Fixed by #29854
Closed

Query/MQ: consider making ExpressionPrinter static #29851

maumar opened this issue Dec 14, 2022 · 1 comment · Fixed by #29854
Assignees
@roji
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone
8.0.0

Comments

@maumar
Copy link
Contributor

maumar commented Dec 14, 2022

Currently the printer is instance based, since it has some state that it needs to property print the expression (e.g. list of encountered variable names so that it can generate unique ones when needed). We could consider making it static, so that it's easier to use. Would have to ditch the visitor pattern, and flow the state between our own "visit" methods. Uglier code to write but easier to use.
roji added a commit to roji/efcore that referenced this issue Dec 14, 2022
@roji
Add static Print methods to ExpressionPrinter
2dbec75 
And some cleanup

Closes dotnet#29851
@roji roji mentioned this issue Dec 14, 2022
Merged
@roji
Copy link
Member

roji commented Dec 14, 2022

Raised in #29808 (comment).

I just meant adding a static method to ExpressionPrinter which internally instantiates ExpressionPrinter - no need to ditch the visitor pattern or anything. Submitted #29854 with a proposal for this.

roji added a commit that referenced this issue Dec 14, 2022
@roji
Add static Print methods to ExpressionPrinter (#29854)
3a62379 
And some cleanup

Closes #29851
@ajcvickers ajcvickers added this to the 8.0.0 milestone Feb 7, 2023
@ajcvickers ajcvickers added area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels Feb 7, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview1 Feb 7, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview1, 8.0.0 Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roji roji

Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Milestone
8.0.0
Development

Successfully merging a pull request may close this issue.

Add static Print methods to ExpressionPrinter roji/efcore
3 participants
@ajcvickers @maumar @roji