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

Use ThrowHelper for exceptions #45

Closed
SteveDunn opened this issue Jan 4, 2022 · 9 comments
Closed

Use ThrowHelper for exceptions #45

SteveDunn opened this issue Jan 4, 2022 · 9 comments
Labels

Comments

@SteveDunn
Copy link
Owner

No description provided.

@SteveDunn
Copy link
Owner Author

I'm still looking at this one, although I haven't been able to find a suitable bit of code that I can benchmark (in benchmark.net) that demonstrates any speed increase.

@SteveDunn SteveDunn added the good first issue Good for newcomers label Jun 18, 2022
@SteveDunn
Copy link
Owner Author

I'm still looking at this one, although I haven't been able to find a suitable bit of code that I can benchmark (in benchmark.net) that demonstrates any speed increase.

Profiling this is quite tricky, although there are performance gains to be had, so this item should be implemented.

@Herdo
Copy link
Contributor

Herdo commented Aug 29, 2022

@SteveDunn What's the purpose for this? Maintainability or performance?

@viceroypenguin
Copy link
Contributor

Performance- using a helper method improves the rendered code from JIT.

https://docs.microsoft.com/en-us/windows/communitytoolkit/developer-tools/throwhelper#technical-details

@Herdo
Copy link
Contributor

Herdo commented Aug 29, 2022

@viceroypenguin 👍 The ThrowHelper methods from CommunityToolkit.Diagnostics would only be used for the system exceptions then. Is that the only purpose, or should the ValueObjectValidationException receive a wrapper method as well, to benefit from the performance benefits?

@viceroypenguin
Copy link
Contributor

Yes, other exceptions should be wrapped separately with a similar helper class to benefit from similar performance optimizations. All exceptions receive these optimizations, not just this library or the system exceptions - it's just that the system ones are the ones that make sense to put into CommunityToolkit.Diagnostics. Whether @SteveDunn wants to add a dependency to CommunityToolkit.Diagnostics or copy the relevant exceptions independently for Vogen is up to him.

@SteveDunn
Copy link
Owner Author

Looking at this one at long last! I don't really want to take a dependency on anything, so I'm thinking of generating a type that has all of the methods required for the exceptions that are thrown with Vogen.
I'm just wondering if this is something that has been, or is being, implemented in .NET 9.

@viceroypenguin
Copy link
Contributor

Looking at this one at long last! I don't really want to take a dependency on anything, so I'm thinking of generating a type that has all of the methods required for the exceptions that are thrown with Vogen. I'm just wondering if this is something that has been, or is being, implemented in .NET 9.

For Vogen's case, I'd recommend having an internal static class with the helper methods. ThrowHelper or similar. In this case, I agree with avoiding the dependency on CommunityToolkit.Diagnostics, and you'll have to create your own methods for the Vogen exceptions anyway. Just my 2c.

@SteveDunn SteveDunn mentioned this issue Sep 24, 2024
@SteveDunn
Copy link
Owner Author

Implemented in 5.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants