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

VB -> C#: Improve query syntax support #129

Merged
merged 14 commits into from
Aug 20, 2018
Merged

Conversation

GrahamTheCoder
Copy link
Member

@GrahamTheCoder GrahamTheCoder commented May 29, 2018

#29

Problem

Currently don't handle several keywords, or nested statements

Solution

  • First step was to handle nested statements by chunking sections together.

  • At least one test covering the code changed

  • All tests pass

Which part of this PR is most in need of attention/improvement:

  • The higher level methods in the QueryConverter are pretty much write-only code. They just batch up bits of the query for other methods to handle later, but are missing appropriate naming and probably a data structure of their own.
  • The lower level methods still don't handle many cases, currently they will just throw an exception due to using Single in places where there can be multiple elements - I don't have examples of the more advanced cases, so I think this is fine for now, and better than barreling on missing information without informing the user.

@GrahamTheCoder GrahamTheCoder self-assigned this May 29, 2018
@GrahamTheCoder
Copy link
Member Author

I think I'd like to write a few more tests to get this bit merged before moving on to the other points. I'm slightly worried some cases may be worse on this branch still

@GrahamTheCoder GrahamTheCoder merged commit 18e8ccf into master Aug 20, 2018
@GrahamTheCoder GrahamTheCoder deleted the parse-query-syntax branch August 20, 2018 10:58
@GrahamTheCoder GrahamTheCoder restored the parse-query-syntax branch August 20, 2018 15:46
@GrahamTheCoder GrahamTheCoder deleted the parse-query-syntax branch June 9, 2019 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant