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

Proposal: Non-Positional Parameters #1478

Closed
bondsbw opened this issue Apr 24, 2018 · 33 comments
Closed

Proposal: Non-Positional Parameters #1478

bondsbw opened this issue Apr 24, 2018 · 33 comments

Comments

@bondsbw
Copy link

bondsbw commented Apr 24, 2018

Positional arguments can be easy to write but difficult to read:

AddStudent("Amanda Jones", true, false, false, true, 10, 5, 10);

To fully understand this call, you need to cross-reference the AddStudent method signature.

public void AddStudent(string name, bool lockerAssigned, bool outOfStateTransfer, bool overwrite, 
   bool throwOnError, int gradeLevel, int retryCount, int timeout)
{
    ...
}

The caller can optionally choose to name the arguments:

AddStudent("Amanda Jones", lockerAssigned: true, outOfStateTransfer: false, overwrite: false, 
    throwOnError: true, gradeLevel: 10, retryCount: 5, timeout: 10);

But this is unenforceable.

Proposal

Allow the method writer to require arguments to be named.

NonPositionalAttribute, applied to a parameter, will require the argument to be named.

public void AddStudent(string name, 
    [NonPositional] bool lockerAssigned,
    [NonPositional] bool outOfStateTransfer,
    [NonPositional] bool overwrite,
    [NonPositional] bool throwOnError,
    [NonPositional] int gradeLevel,
    [NonPositional] int retryCount,
    [NonPositional] int timeout)
{
    ...
}

In this case, only name is allowed to be called without a name, while all the others must be named.

Considerations

  1. NonPositionalAttribute could be applied at the method level to require all arguments to be named. This could be applied even more broadly, such as the class or assembly levels.
  2. When applied at levels higher than argument, allow types to act as a filter. The following would be equivalent to the above example (syntax borrowed from Champion "Allow Generic Attributes" #124):
[NonPositional<bool, int>]
public void AddStudent(string name, bool lockerAssigned, bool outOfStateTransfer, bool overwrite, 
   bool throwOnError, int gradeLevel, int retryCount, int timeout)
{
    ...
}
  1. As mentioned by @HaloFour, this would be unenforceable for previous versions of the language.
@Logerfo
Copy link
Contributor

Logerfo commented Apr 24, 2018

This can be achieved with a custom analyzer. I don't see a reason for this to be added as a new syntax restriction.

@HaloFour
Copy link
Contributor

I agree with @Logerfo , the behavior from the compiler can already be enforced through an analyzer, especially given that your proposal pretty much builds on existing syntax (except where you get into generic attributes, but you can always use a params Type[] constructor parameter instead).

I also think that tooling support in the IDE, either built-in or through an extension, could give this behavior to you for free without requiring any changes to the language/compiler, as mentioned here: #1462 (comment)

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@HaloFour

(except where you get into generic attributes, but you can always use a params Type[] constructor parameter instead)

Right, that is not part of this proposal. I just think it looks nicer. 🙂

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@Logerfo I can't enforce consumers of my API to add my analyzer. Your suggestion would limit applicability.

(At least I don't think I can. Correct me if I'm wrong.)

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@HaloFour I do like the IDE suggestion.

@HaloFour
Copy link
Contributor

HaloFour commented Apr 24, 2018

@bondsbw

I can't enforce consumers of my API to add my analyzer. Your suggestion would limit applicability.

Similarly you couldn't force customers to use a specific version of the C# compiler, and older compilers would be unaware of this attribute and how it should restrict compilation. Or compilers for other languages.

Ultimately I think it should be up to the consumer of any given API to decide if embedding the parameter names at the call site is valuable to them, not the author of said API. I wouldn't use an API that attempted to force my hand in such a manner.

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

Fluent APIs tend to enforce descriptive usage. I use them regularly.

This proposal is a small step toward adding a fluent API but without all the hassle.

@stakx
Copy link
Contributor

stakx commented Apr 24, 2018

Allow the method writer to require arguments to be named.

Why should the method writer care about how some unknown consumer of the method is going to write the method call?

If the method writer really cares, they'd do better by not creating that many parameters in the first place.

Also, the method writer might think his method won't be legible if the arguments aren't explicitly named at the call site... but that isn't necessarily the case. A good choice of variable names can still result in a reasonably understandable call. (For example, no point in forcing the consumer to spell out an argument [NonPositional] T frobbler if he passes a variable frobbler as the argument. frobbler: frobbler at the call site would actually be worse than just frobbler.)

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@stakx I agree, this does look nicer:

AddStudent("Amanda Jones", lockerAssigned, outOfStateTransfer, throwOnError, overwrite, gradeLevel, retryCount, timeout);

Except that I accidentally switched two arguments.

And you probably had to look at the top of this page to see which ones.

@stakx
Copy link
Contributor

stakx commented Apr 24, 2018

@bondsbw - Yes, given that many bool parameters, the problem won't go away completely. More about that in a sec. My main point was that a method writer would perhaps better invest their time in designing a sensible, user-friendly API first before deciding how to force upon consumers a particular call syntax that may not even be desirable in their opinion. It's your responsibility as a method writer to design it as well as you can. Your consumers' choice of coding style should be left up to them.

Since I mentioned API design, here's some possible alternatives that might reduce or even obliterate the need for enforcing named arguments:

  • Use object initializer syntax:

    Add(new Student { LockerAssigned = true, OutOfStateTransfer = false, ... });
  • You might have heard the "Boolean parameters are evil" argument. In a similar vein, perhaps replace some or all of your bool parameters with enum ones to make mixups more obvious. Or, instead of using plain ints everywhere, use custom structs that convey additional meaning and use the type system to prevent accidental mixups: You won't be able to accidentally put an Area-typed argument in a Distance-typed parameter, even though both might simply be wrappers around the same primitive type (e.g. double).

There's probably many more options. What I'm saying is that a not-to-well-designed method (IMHO) like AddStudent("Amanda Jones", true, false, false, true, 10, 5, 10); is not an ideal starting point for requesting this new language feature.

@stakx
Copy link
Contributor

stakx commented Apr 24, 2018

P.S.: In case this feature request gains traction, I'd be happy if at least the compiler deemed e.g. frobbler a valid short form for frobbler: frobbler for enforced non-positional params, to avoid unnecessary redundancy.

@DavidArno
Copy link

The crappiness quotient of a method can be expressed as (number of parameters)^(number of boolean parameters+1), with 1 being "not at all crappy" and this example, of 8^5, or 4096, is pretty damned crappy.

I'm with @stakx here: don't fix this with compiler bodges; fix it by reducing the number of parameters.

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@stakx

I'm sure you've heard the "Boolean parameters are evil" argument.

Of course. That's the point of this proposal, reducing hidden intent.

The problem goes beyond Booleans and includes most types of constants. We can't tell what those integers mean any more than we can tell what the Booleans mean.

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@stakx

My point was that a method writer would perhaps better invest their time in designing a sensible, user-friendly API

That's the other goal of this proposal, KISS.

Alternatives inevitably require more stuff. Maybe you need an enum for your flags. Or a class to hold all the parameters. Or more methods (and possibly more types) to create a fluent API.

Those all work great, but they aren't as user-friendly.

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@stakx

  • Use object initializer syntax:
Add(new Student { LockerAssigned = true, OutOfStateTransfer = false, ... });

That only works if all the parameters are optional.

@HaloFour
Copy link
Contributor

I'm going to note that I'm not particularly arguing against the idea of requiring the parameter names at the call site. I think it's a good debate to have when designing v1 of a language. But we're leading into v8, positional parameters are the standard and there's nothing that you can do as an API author to force any consumer of your API to actually use a version of the compiler that can enforce such a behavior. That reason, above all others, is why I don't feel that this proposal is not a good fit for C#.

@bondsbw
Copy link
Author

bondsbw commented Apr 24, 2018

@HaloFour

there's nothing that you can do as an API author to force any consumer of your API to actually use a version of the compiler that can enforce such a behavior

This is somewhat minor (IMHO). Existing behavior is a reasonable fallback.

@HaloFour
Copy link
Contributor

@bondsbw

This is somewhat minor (IMHO). Existing behavior is a reasonable fallback.

Which then breaks if the user updates the compiler, even if they are entirely unaware of whether various APIs they consume might try to enforce this behavior.

@Opiumtm
Copy link

Opiumtm commented Apr 25, 2018

The best solution to make method parameters a less mess is a struct which would hold all the parameters and embed all required parameter logic into the struct. So, by introducing a struct you gain

  1. Better level of backward compatibility. You can totally avoid lots of method overloads if you introduce additional parameters or change method signature.
  2. Enforce required positional parameters in non-default struct constructor and optional non-positional parameters as struct fields that aren't initialized by constructor.
  3. You could have additional logic embedded into the struct to validate or correct parameters even before struct is passed to method as argument.
  4. Structs introduce little overhead and doesn't allocate unnecessary memory.

I often use structs for public APIs parameters instead of long parameter lists.

@bondsbw
Copy link
Author

bondsbw commented Apr 25, 2018

@Opiumtm As stated above, it's more work for the implementer and still not a solution for required parameters.

@Opiumtm
Copy link

Opiumtm commented Apr 25, 2018

@bondsbw Required parameters could be at non-default struct's constructor, of course.

@bondsbw
Copy link
Author

bondsbw commented Apr 25, 2018

@Opiumtm It misses the point of this proposal. There is no way to require the caller to specify those argument names.

@theunrepentantgeek
Copy link

As stated above, it's more work for the implementer and still not a solution for required parameters.

It's more work for the implementer once.
It's less opportunity for mistakes every time a developer writes code to call the method.
It's easier comprehension any time a developer reads the method call in the future.

A struct with readonly values for required parameters, initialized from a constructor, handles required parameters quite well, I believe.

Note also that there's no requirement for everything to be contained in the one struct.

Take your original example:

public void AddStudent(string name, bool lockerAssigned, bool outOfStateTransfer, bool overwrite, 
   bool throwOnError, int gradeLevel, int retryCount, int timeout)
{
    ...
}

One possible restructuring of this would be:

public void AddStudent(NewStudentInformation student, ServiceOptions options)
{
    …
}

public struct NewStudentInformation
{
	public string Name { get; }
	public bool LockerAssigned { get; set; }
	public bool OutOfStateTransfer { get; set; }
	public int GradeLevel { get; set; }
	
	public NewStudentInformation(string name) { ... }	
}

public struct ServiceOptions
{
    public bool Overwrite { get; set; }
	public bool ThrowOnError { get; set; }
	public int RetryCount { get; set; }
	public int Timeout { get; set; }
}

In this scenario, it's likely that ServiceOptions wouldn't be constructed anew for each call; I'd create a standard one that I passed in wherever required, only creating a custom one when the requirements for that service call were different.

@bondsbw
Copy link
Author

bondsbw commented Apr 26, 2018

@theunrepententgeek

How does your example force the caller to supply parameter names for required parameters?

That's the purpose of this proposal, so failing to address that specific need is pointless.

@DarthVella
Copy link

Because the NewStudentInformation and ServiceOption bool/int settings are not part of their respective constructors, and so must be explicitly set.

var newStudent = new NewStudentInformation("James Smith")
{
   LockerAssigned = true,
   GradeLevel = 9
};

etc, etc.

@theunrepentantgeek
Copy link

@bondsbw I think you're missing my point, so let me be more explicit.

I think this proposal provides an approach for papering over problems in badly designed method signatures.

  1. It doesn't actually address the underlying problem of bad design; it just provides a way for bad designs to be "justified" as usable instead of fixed.
  2. It places a higher burden on every consumer of the API instead of putting the onus on the method author to make things simple to use.

I'd suggest that enhancements to C# should primarily fall into one of two categories: features that make the language more powerful and expressive (async/await, shapes, records, etc) or that encourage good development practices (e.g. expression bodied members encouraging short implementations). I don't think proposal falls into either category.

@bondsbw
Copy link
Author

bondsbw commented Apr 27, 2018

@DarthVella Those named parameters are not required. You left some out, demonstrating my point.

@theunrepentantgeek This proposal is intended to fix the bad part. I don't feel this is a problem in Objective-C for example, where parameter names are required.

I feel this squarely meets your second criteria by encouraging readable method calls, and without extra structs which require more typing just to "fix" this problem for the subset of cases where the parameters are optional.

@HaloFour
Copy link
Contributor

I don't feel this is a problem in Objective-C for example, where parameter names are required.

This was a part of the initial design of Objective-C, not added in a much later version. It's interesting that in Swift they make it possible to define functions that don't require the argument labels.

@bondsbw
Copy link
Author

bondsbw commented Apr 27, 2018

Some who come from an Obj-C background feel that aspect of Swift was a mistake.

That said, I concede that there is little support for this in C#.

@bondsbw bondsbw closed this as completed Apr 27, 2018
@DarthVella
Copy link

@bondsbw The example I gave relied on the default for OutOfStateTransfer being false, but I'll concede the point.

@bondsbw
Copy link
Author

bondsbw commented Apr 30, 2018

Coincidentally, I just found out via a mailing list that Python added this concept in version 3.0.

https://www.python.org/dev/peps/pep-3102/

@HaloFour
Copy link
Contributor

@bondsbw

Coincidentally, I just found out via a mailing list that Python added this concept in version 3.0.

Sounds nearly identical to #1458.

@bondsbw
Copy link
Author

bondsbw commented Apr 30, 2018

Indeed, if we really were trying to copy the concept from Python 3 then this proposal might extend #1458 in this way:

public void AddStudent(string name, params, bool lockerAssigned, bool outOfStateTransfer, 
    bool overwrite, bool throwOnError, int gradeLevel, int retryCount, int timeout)
{
    ...
}

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

No branches or pull requests

8 participants