-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Guard APIs #3131
Guard APIs #3131
Conversation
Thanks Sergio0694 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
This PR has been marked as "needs attention 👋" and awaiting a response from the team. |
Note to self: Squash this commit as per @azchohfi's recommendation. @Sergio0694 thanks for this amazing addition! I'm just going to do a quick final play locally with it in the morning and then will merge the PR! |
@Sergio0694 seeing 4 test cases fail? IsCloseToFloat_Fail |
@michael-hawker Good catch! Fixed in 51b45e2 and 244da99.
All are passing now 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Wo-hoo! 😄 🚀 Thank you so much @michael-hawker - so happy to have my first feature PR merged in! |
## Follow up for #3131 <!-- Add a brief overview here of the feature/bug & fix. --> The `Type.ToTypeString()` introduced with the `Guard` API lacks support for nested types, pointer types and byref types. ## PR Type What kind of change does this PR introduce? <!-- Please uncomment one or more that apply to this PR. --> <!-- - Bugfix --> <!-- - Feature --> <!-- - Code style update (formatting) --> <!-- - Refactoring (no functional changes, no api changes) --> <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying, or link to a relevant issue. --> The extension doesn't work properly when used with nested, pointer or byref types. It will either crash or just return the wrong formatted type (eg. the declaring type instead of the nested type). ## What is the new behavior? <!-- Describe how was this issue resolved or changed? --> The extension now works correctly with all sorts of nested, pointer and byref types 🎉 ## PR Checklist Please check if your PR fulfills the following requirements: - [X] Tested code with current [supported SDKs](../readme.md#supported) - [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs).~~ - [ ] ~~Sample in sample app has been added / updated (for bug fixes / features)~~ - [ ] ~~Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~ - [X] Tests for the changes have been added (for bug fixes / features) (if applicable) - [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [X] Contains **NO** breaking changes
PR Type
What kind of change does this PR introduce?
What is the new behavior?
This PR introduces the
Guard
class, which can be used to validate method arguments in a much more streamlined manner, which is also less verbose and less error prone.These APIs are built with three core principles in mind:
Guard
API will (almost always) be inlined. Furthermore, specialized methods are generated with T4 templates to achieve the most efficient assembly code possible when working with common types (eg. primitive numeric types).Guard
API produces a detailed exception message that clearly specifies what went wrong, along with additional info (eg. current variable values), when applicable.Guard
APIs have expressive and self-explanatory names that make it immediately clear what each API is supposed to do. This shifts the burden of writing checks away from the developers, letting them express conditions using natural language.Here is a sample of how these APIs greatly reduce the complexity of code and make it easier to write and to follow, while also making it less error prone:
Below is a breakthrough of most of the new types and APIs added in this PR (there are more):
Click to expand!
There are also a couple of new extension methods being introduced in this PR, which are used internally. Since they were pretty general and easily reusable, they have been marked as public:
PR Checklist
Please check if your PR fulfills the following requirements:
Sample in sample app has been added / updated (for bug fixes / features)Icon has been created (if new sample) following the Thumbnail Style Guide and templatesOther information
This PR is a follow up from a conversation with @michael-hawker over on the UWP Community Discord server. The
DebugClass
is missing for now, but since it should be exactly identical to theGuard
class, just with the added[Conditional("DEBUG")]
attribute, I was thinking we could probably use a T4 template (or some similar technique) to just auto-generate this class by copying theGuard
code and adding that single attribute to the new class. I'm not personally familiar with setting up a T4 template, so I would need some help to get that figured out. I'll work on theGuard
class in the meantime 👍