How to ensure compilation error on signature change where 'params' keyword is used
Asked Answered
P

2

9

I have a method like this:

public void Foo(params string[] args) {
  bar(args[0]); 
  bar(args[1]);
}

The new requirements lead to a change like this:

public void Foo(string baz, params string[] args) {
  if("do bar".Equals(baz)) {
    bar(args[0]); 
    bar(args[1]);
  }
}

The problem is that even though I've changed the method signature, no compilation errors occur, which is correct of course, but I want there to be compilation errors for every call to Foo method where the argument baz has not been specified. That is, if a call to Foo before the change was this one:

Foo(p1,p2); //where p1 and p2 are strings

it now needs to be this one:

Foo(baz,p1,p2);

If it wouldn't be changed in this way, p1 would be assigned to baz, and the params array args would be of length 1 and an OutOfBounds exception would be thrown.

What's the best way to change the signature and ensure that all the calling code is updated accordingly? (The real scenario is where Foo lives in an assembly shared by many projects automatically built on a build server. A compilation error would thus be an easy way to detect all the code that needs to be touched to accomodate the change.)

Edit: As Daniel Mann and others pointed out, the example above suggests that I should not use params at all. So I should explain that in my real world example it's not always the case that args needs to have two elements, as far as the logic in Foo is concerned args can contain any number of elements. So let's say this is Foo:

public void Foo(string baz, params string[] args) {
  if("do bar".Equals(baz)) {
    int x = GetANumberDynamically();
    for(int i = 0; i<x; i++)
      bar(args[i]); 
  }
}
Philender answered 5/12, 2013 at 13:0 Comment(6)
Why not change the signature to do something completely different (like to take an int or something). Then a compile error will be thrown on all the methods? Of course a better way would be to refactor with Resharper or similar.Dower
To add to @dav_i: if you first add an int instead of the string, compile, modify all calling code, then change the int to string. Also note that if("do bar".Equals(baz)) { is a really bad example, it sounds like another overload or more refactoring is required.Boyhood
Do what @OndrejJanacek suggests. It's definitely the best way.Dower
Just to be sure: Does your params-parameter need EXACTLY two inputs or AT LEAST two inputs? In the former case I would suggest not using params at all...Puritanical
@Dower The reason I don't prefer this is that the maintenance for the depending projects are spread out over different persons. If suddenly they get a build error on their table due to an int in the signature it can be confusing and not obvious what they need to do. What I want is a compilation error that require a fix that is the permanent solution. But if this is not achievable I think I will go with your suggestion.Philender
@Puritanical Actually no, that is just the contrived example. I should have explained this. From the context of Foo it's not known how many params-parameters args should contain to avoid an exception.Philender
A
3

Here's the solution. Do not change the former method signature, just add the Obsolete attribute with both arguments specified.

[Obsolete("Use Foo(string, params string[]) version instead of this", true)]
public void Foo(params string[] args) {
  bar(args[0]); 
  bar(args[1]);
}

Then create a new method with a new signature.

public void Foo(string baz, params string[] args) {
  if("do bar".Equals(baz)) {
    bar(args[0]); 
    bar(args[1]);
  }
}

The second argument in the Obsolete attribute ensures a compilation error. Without it it just causes a compilation warning. More info about the attribute is available on MSDN.

EDIT:

Based on discussion in comments below, Daniel Mann came up with an interesting problem.

That wouldn't solve the problem. What about if you call Foo("a", "b")? In that case, it will still call the non-obsolete method with only two arguments, and cause the same problem.

I would advise to check if there is more then one argument passed through args before calling bar.

Advocation answered 5/12, 2013 at 13:24 Comment(8)
That wouldn't solve the problem. What about if you call Foo("a", "b")? In that case, it will still call the non-obsolete method with only two arguments, and cause the same problem.Hausmann
Yes, it will choose the new overload, and then the args array will only have one item in it, which is still going to cause a runtime error in the example provided. The original question was about how to avoid that sort of problem.Hausmann
Yeah, but that is not my problem. That's just a bad design where OP assumes that there will be always two arguments passed through args.Advocation
It's your problem inasmuch as the answer you gave doesn't actually answer the question.Hausmann
Well, it is not that often that I come up with every corner case. That's why there are other people and comments. I'll update answer appropriately, and thank you for pointing out.Advocation
Thanks for the input. As pointed out this does not solve the example given: calling Foo with two strings.Philender
Read my edit part of the question. Even without this problem in the question, you should not rely on the number of provided arguments. Better to check it before using it. I believe that combination of my solution with checking for the number of arguments provides the solution.Advocation
What I am trying to get as is how to enforce calling code to provide the new parameter baz. I don't see how this would be solved by adding checks within Foo.Philender
H
2

The easiest solution is to not use the params keyword if you have required parameters.

Obviously, you're expecting args to contain at least two parameters. It's safe to say that those are required. Why not have a method signature like this?

public void Foo(string baz, string requiredArgument1, string requiredArgument2, params string[] optionalArguments)

That removes the ambiguity: It will always require at least 3 arguments.

Another option I hadn't even thought of for some reason is to use named parameters. Obviously, all of your code would have to explicitly do so, but you could do this:

Foo(baz: "bar", args: new [] {"a", "b", "c"});

Hausmann answered 5/12, 2013 at 13:52 Comment(3)
You are completely right regarding my dummy code example. Unfortunately in my real world example it's not known from the context of Foo how many strings are required. (The args are used for parameters for queries that are loaded dynamically.)Philender
If Foo handles many different contexts, then perhaps it may be doing too much? I still think that a more comprehensive redesign of the method signature may be needed. At this point maybe you could shed more light on the actual intended usageChromatics
@JunWeiLee It doesn't handle different contexts, just a dynamic one. The actual usage is such that the params string[] args end up being parameters in a a query. Which query/the number of parameters is only known at runtime.Philender

© 2022 - 2024 — McMap. All rights reserved.