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

Updated AV2310 for local functions #155

Closed
wants to merge 1 commit into from
Closed

Updated AV2310 for local functions #155

wants to merge 1 commit into from

Conversation

bkoelman
Copy link
Contributor

@bkoelman bkoelman commented May 7, 2018

No description provided.

@bkoelman bkoelman mentioned this pull request May 7, 2018
16 tasks
@@ -20,7 +20,7 @@ Following the MSDN online help style and word choice helps developers find their
**Tip:** The tool [GhostDoc](http://submain.com/products/ghostdoc.aspx) can generate a starting point for documenting code with a shortcut key.

### <a name="av2310"></a> Avoid inline comments (AV2310) ![](/assets/images/2.png)
If you feel the need to explain a block of code using a comment, consider replacing that block with a method with a clear name.
If you feel the need to explain a block of code using a comment, consider replacing that block with a method or local function with a clear name.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this one. If the code becomes that much unreadable, a local function will not really help focusing on the current body. In that case, I would prefer to have a separate method or to properly encapsulate a concept that is now represented by primitive types. See also https://www.continuousimprover.com/2015/10/9-simple-practices-for-writing-better.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Let's discard this PR then.

@bkoelman bkoelman closed this May 10, 2018
@bkoelman bkoelman deleted the local-av2310 branch May 10, 2018 09:13
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.

2 participants