Closed bkoelman closed 7 years ago
I'm not convinced yet. The goal of this rule is to make sure you can understand the high-level purpose without the need to understand the details of the algorithm. By having local functions, I think you'll be creating a lot of noise.
Good point. Using a local function here reduces the type complexity (less members), but at the cost of making the method more complex. So let's close this issue and keep things as-is.
So maybe we need a guideline to avoid using local functions.
Are you seeing that your team members are misusing them? I'd rather not advice against using new features, but instead identify where they are useful. Like precondition checks in iterators and async methods, and the cases where Resharper suggests them. It just happened I used one today, which I'm happy with. https://github.com/bkoelman/RoslynTestFramework/commit/635a39091c2b6ea6642af4545752fdf9e497a5c8#diff-590794906c8412cd8a13f186d62e349b
Maybe local functions need a bit of time to find their place. Years ago I found people advising against Linq, lambda's, generics etc because devs would not understand them. Or factory methods, because it makes construction non-obvious. I trust the language designers take great care in extending, also looking at other platforms. Nested functions in JavaScript (the most widely used language) are very common.
Existing rule:
Encapsulate complex expressions in a method or property (AV1547)
Consider the following example:
[example]
In order to understand what this expression is about, you need to analyze its exact details and all of its possible outcomes. Obviously, you can add an explanatory comment on top of it, but it is much better to replace this complex expression with a clearly named method:
[example]
You still need to understand the expression if you are modifying it, but the calling code is now much easier to grasp.
New language features to consider: