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

try-catch and using statements in exception handling #31951

Closed
bitm0de opened this issue Oct 20, 2022 · 2 comments · Fixed by #36428
Closed

try-catch and using statements in exception handling #31951

bitm0de opened this issue Oct 20, 2022 · 2 comments · Fixed by #36428
Assignees
Labels
doc-enhancement Improve the current content [org][type][category] dotnet-csharp/svc fundamentals/subsvc Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.

Comments

@bitm0de
Copy link

bitm0de commented Oct 20, 2022

It says, "Use a try-catch statement for most exception handling" and proceeds to show an example where the exception could be avoided by doing bounds checks. I don't think this should be a recommended coding convention since exception-based programming is more expensive than mitigating exceptions that can be prevented.

If anything, this should demonstrate catching an exception that you can't prevent (i.e. SocketException in some cases), and not catching the System.Exception type but the most relevant derived exception as a best practice.


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.


Associated WorkItem - 123090

@issues-automation issues-automation bot added fundamentals/subsvc dotnet-csharp/svc Pri1 High priority, do before Pri2 and Pri3 labels Oct 20, 2022
@dotnet-bot dotnet-bot added the ⌚ Not Triaged Not triaged label Oct 20, 2022
@BillWagner BillWagner added the doc-enhancement Improve the current content [org][type][category] label Oct 24, 2022
@dotnet-bot dotnet-bot removed the ⌚ Not Triaged Not triaged label Oct 24, 2022
@BillWagner
Copy link
Member

These are generally good suggestions @bitm0de

I'm not convinced that SocketException or something similar is the best option. I'd rather pick something more common, to avoid the need to explain more about the conditions which cause the exception.

@bitm0de
Copy link
Author

bitm0de commented Oct 24, 2022

These are generally good suggestions @bitm0de

I'm not convinced that SocketException or something similar is the best option. I'd rather pick something more common, to avoid the need to explain more about the conditions which cause the exception.

You mention that it is not the best option, although no alternative suggestion is made. SocketException is rather common in networking, and an exhaustive list of preconditions doesn't have to be provided to make a point. One probable cause is to state the closing of the connection from the server side. The real point being, to use a try-catch as the solution in an appropriate context for catching the exception (i.e. when you can't mitigate it in most circumstances).

The current example provides no explanation about what would cause an IndexOutOfRangeException, or whether it's the most appropriate way to handle the underlying cause of the exception rather than preventing it. Thus, it does seem odd to suggest the need for listing the causes for a SocketException, even if it is arguably a better use of a try-catch.

@BillWagner BillWagner self-assigned this Jun 30, 2023
@BillWagner BillWagner added the 🗺️ reQUEST Triggers an issue to be imported into Quest. label Jun 30, 2023
@github-actions github-actions bot added 📌 seQUESTered Identifies that an issue has been imported into Quest. and removed 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Jul 1, 2023
BillWagner added a commit to BillWagner/docs that referenced this issue Jul 27, 2023
Fixes dotnet#31951 : Rewrite the exception example so that it's still obvious what can fail, but couldn't be easily anticipated before making the computation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-enhancement Improve the current content [org][type][category] dotnet-csharp/svc fundamentals/subsvc Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants