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.

Sign up for GitHub

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

Jump to bottom

AV1561: Update for tuples and local functions #104

Closed
bkoelman opened this issue Oct 10, 2017 · 6 comments
Closed

AV1561: Update for tuples and local functions #104

bkoelman opened this issue Oct 10, 2017 · 6 comments

Comments

@bkoelman
Copy link
Contributor

Existing rule:

Don't allow methods and constructors with more than three parameters (AV1561)
If you create a method with more than three parameters, use a structure or class to pass multiple arguments, as explained in the [Specification design pattern](http://en.wikipedia.org/wiki/Specification_pattern). In general, the fewer the parameters, the easier it is to understand the method. Additionally, unit testing a method with many parameters requires many scenarios to test.

New language features to consider:

  • tuples
    The next method signature accepts five values, but would be allowed by this rule in its current form:
public void SavePerson((string firstName, string middleName, string lastName, string nickname, DateTime dateOfBirth) personData)
{
    // ...
}

However, blocking usage of tuples seems a bit too strict to me. Perhaps allow tuples, but limit their maximum number of items? Alternatively, count the tuple items separately? That would allow a method with two parameters, where the second parameter is a tuple with two items.

  • local functions
    This rule should probably also apply for local functions and delegates. We could change the first line to:
    If you create a method, local function or delegate with more than three parameters...
@bkoelman bkoelman mentioned this issue Oct 10, 2017
Closed
20 tasks
@bkoelman
Copy link
Contributor Author

I believe the essence of this rule is to limit the number of values that are passed to a method. The goal of that would be to keep the method body simple. And to prevent flooding the readers' mind with a large set of states (variables, parameters, fields) to be tracked, while analyzing the execution flow.

If the goal is to limit the number of values to be tracked, then maybe that should also apply for the return value (now that we can have multiple). Perhaps this rule should be rephrased to constrain the total number of values going in and out. If set to 4, that would allow:

public (string first, string second) DoWork(string input, string other);

but not:

public (string first, string second, string third) DoWork(string input, string other);

@dennisdoomen
Copy link
Owner

That makes sense indeed. In fact, a tuple with more than two values should be frowned upon.

@bkoelman bkoelman mentioned this issue Oct 25, 2017
Merged
@dennisdoomen
Copy link
Owner

Another thing to consider are the naming conventions as @boekabart asked about in his gist. After reading this article I tend to lean towards this:

public (string First, string Second) DoWork(string input, string other);

So names of the tuple members use Pascal casing when returning them .

Why do you think @bkoelman ?

@bkoelman
Copy link
Contributor Author

bkoelman commented Feb 2, 2018 via email

@dennisdoomen
Copy link
Owner

dennisdoomen commented Feb 2, 2018 via email

@bkoelman
Copy link
Contributor Author

bkoelman commented Feb 2, 2018 via email

@bkoelman bkoelman mentioned this issue May 3, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dennisdoomen @bkoelman