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

Feature: Parameter Name Aliasing #7

Closed
am11 opened this issue Jan 15, 2015 · 8 comments
Closed

Feature: Parameter Name Aliasing #7

am11 opened this issue Jan 15, 2015 · 8 comments
Assignees
Labels
Area-Language Design Feature Request Language-C# Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. Verification Not Required
Milestone

Comments

@am11
Copy link
Member

am11 commented Jan 15, 2015

The human meaning of parameters may vary from method caller's perspective.

It would be useful if the language provides an ability to assign aliases to the parameters.

[Alias({second: [secondAndHalf, quarterToThree], first: [Initial]})] 
public static void Member1(string first, string second) { 
  // Here the user will always use the given names: first and second.
  // which means the user cannot use the param by its nickname
  // inside the method scope.
}

private static void Caller1() {
  Member1(secondAndHalf: "secondly, this is not a bad idea after all",
          Initial: "Initially it sounded moot... :)  ");
}

private static void Caller2() {
  Member1(first: "1st",
          quarterToThree: "almost there");
}

private static void Caller3() {
  Member1("one", "two");
}

vs. this current approach:

public static void Member1(string first, string second) {  }

public static void Member1(string first, string secondAndHalf, string devNull = null) { 
  Member1(first, secondAndHalf);
}

Note: the parameter devNull may violate CA1026, in case of overridden methods. On the flip side, setting devNull as non-default parameter would yield "unused parameter".

Related / Real world example: madskristensen/WebEssentials2013#380 (comment)

@mirhagk
Copy link

mirhagk commented Jan 15, 2015

One important thing to note is that this allows you to refactor functions without breaking existing callers. This solves some of the biggest issues around a few suggested features (primary constructors) that required the parameter names to change (which only affects those who call it by name).

I'm not sure how it would be exposed in the BCL. The two ways I can see that allow backwards compatibility are:

  1. Generate 1 method per combination that forwards to the correct method, ie:
public static void Member1(string Initial, string secondAndHalf){Member1(Initial,secondAndHalf);}
public static void Member1(string first, string secondAndHalf(Member1(first,secondAndHalf);}
....

This would work, but generate a lot of clutter methods (In this case it'd be 6 methods, others could easily see much more).

  1. Generate 1 forwarding method that has all named parameters. e.g:
public static void Member1(string first = null, string second=null, string Initial = null, string secondAndHalf = null, string quarterToThree = null){
    Member1(first??Initial,second??secondAndHalf??quarterToThree)
}

This generates only a single extra method. It does rely on the user only specifying one named parameter for each real paramter, and has some extra cost, but should be easily optimized by the CLR. The method should probably have [MethodImplAttribute(MethodImplOptions.AggressiveInlining)] and perhaps a new attribute should be created [Generated(GeneratedOptions.Hide)] so that the method does not appear in intellisense (older compilers would ignore this and show it anyway, allowing consumers to use the named parameters as is, while new compilers would show the alias from the other method).

@svick
Copy link
Contributor

svick commented Jan 15, 2015

I'm not sure how it would be exposed in the BCL. The two ways I can see that allow backwards compatibility are:

Why not just keep the one method with attributes and let the compiler do all the work? The CLR doesn't need to be involved at all.

Also, I don't think your option 1 would work. In .Net, methods can't be overloaded just by parameter names, because only parameter types are specified when calling a method in IL.

@jaredpar jaredpar removed the Language label Jan 15, 2015
@jaredpar jaredpar modified the milestone: Unknown Jan 15, 2015
@mirhagk
Copy link

mirhagk commented Jan 15, 2015

Also, I don't think your option 1 would work.

Yes you are right, that's my own brain fart.

The only problem I see with letting the compiler do all the work is that previous compilers wouldn't be able to compile against the DLL anymore (it would only look at the actual parameter names). If it's okay to sacrifice that bit of backwards compatibility then I agree do everything with the compiler (and translate the named parameters at compile time).

@theoy theoy modified the milestone: Unknown Jan 16, 2015
@MadsTorgersen
Copy link
Contributor

Thanks for the suggestion!

If the main scenario of this is being able to change the names of parameters in a backwards compatible way, then I'm not sure this is worth the cost of a language feature - in terms of added language complexity.

Are there other good scenarios? Please post them here! Maybe I'm missing something valuable.

@am11
Copy link
Member Author

am11 commented Jan 17, 2015

@MadsTorgersen,

Backward compatibility will certainly make things complicated here.

It is a syntactic sugar to state the intent, in pursuit of keeping the code compact and bring about DRY.

This decorator may replace the nicknames with the original counterparts at compile time and keep the IL isolated (from capturing this incident).

The idea really is to explicitly tag the multifaceted parameter, without specifying the code comment at caller to describe its purpose.

@HaloFour
Copy link

I'd have to question the design of these methods if the intention of the arguments can change based on the caller. And if the caller is what determines that intent, why would those "aliases" be dictated by the method? If you discover a new intent you'd be stuck unless you modify the attribute decorating the method.

I don't see why you can't just accomplish this through comments:

Member1(/*Initial*/ "Initially it sounded moot... :)  ",
        /*secondAndHalf*/ "secondly, this is not a bad idea after all");

Member1(/*first*/ "1st",
        /*quarterToThree*/ "almost there");

tmat added a commit that referenced this issue Jan 31, 2015
1) Change MethdCompiler.BindMethodBody to associate correct syntax with BoundBlocks it creates when binding constructor with constructor initializer call (two bound blocks are created – outer one defines a closure scope for constructor parameters, the inner one defines a closure scope for variables in the body).
2) Introduce MethodDebugId – a method ordinal and generation ordinal pair
3) Introduce LamdbaDebugInfo and ClosureDebugInfo to represent information (syntax offset) we use to figure out how to map lambdas and closures to the previous generation.
4) Adds a new PDB CDI record (#7) to store lambda and closure debug info.
5) Generalizes CalculateLocalSyntaxOffset to handle positions in field/property initializers and constructor initializers. Use it to calculate syntax offsets of lambdas and closure scopes. (TODO: rename CalculateLocalSyntaxOffset to CalculateSyntaxOffset).
6) Replace lambda and scope ordinal dispenser integers with array builders that collect LambdaDebugInfo and ClosureDebugInfo.
7) Use TryGet- pattern for all VariableSlotAllocator APIs.
8) Implements mapping of lambda method and display class names to previous generation via VariableSlotAllocator.
 (changeset 1407240)
@gafter gafter added Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. Verification Not Required labels Aug 26, 2015
@gafter
Copy link
Member

gafter commented Aug 26, 2015

Closing, as I cannot imagine anyone on the extended design team championing this over other work. @MadsTorgersen please reopen if you disagree.

@daiplusplus
Copy link

daiplusplus commented Jun 5, 2017

I like this proposed feature based on my experiences with Swift and Objective-C where parameters have separate "parameter name" and "argument labels" (which better translates to "internal names and external names" in my opinion).

This is most useful if you need to use verbose parameter names for external callers but inside the function scope those long names get tedious, for example:

public void ExecuteBankTransfer(Int32 fromAccountId, Int32 toAccountId, Decimal transactionAmount)

I advocate using an inline syntax for alternative names, instead of the OP's use of attributes, so this would become:

public void ExecuteBankTransfer(Int32 fromAccountId from, Int32 toAccountId to, Decimal transactionAmount amount)

...The external caller would still specify fromAccountId: toAccountId: and transactionAmount: but the author of the function definition would use the more succint from, to and amount names.

I find Swift and Objective-C's mandatory labels a bad design choice (though Swift allows the use of underscores to denote optionals), but using a triple Type externalName [internalName] syntax is succint, easy to read, and doesn't conflict with any existing or proposed syntax constructs I'm aware of.

As a bonus, this could be combined with a possible future Record-type syntax, to ensure the public member names and parameter names have the preferred names (mostly thinking of PascalCase vs camelCase):

public class EmployeeRecord(Int32 id Id, String employeeName Name, Image photo Photo);

Becomes:

public class EmployeeRecord {
    public Int32  Id { get; set; }
    public String Name { get; set; }
    public Image Photo { get; set; }
    public EmployeeRecord(Int32 id, String employeeName, Image photo) {
        this.Id = id;
        this.Name = employeeName;
        this.Photo = photo;
    }
}

JoeRobich pushed a commit that referenced this issue Dec 16, 2019
Add tests for: Update 'Populate switch' to support switch-expressions.
Cosifne added a commit that referenced this issue Sep 28, 2021
add two missing code style options to ECUI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Language Design Feature Request Language-C# Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. Verification Not Required
Projects
None yet
Development

No branches or pull requests