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

Question

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]); 
  }
}

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. — dav_i, Dec 05 '13 at 13:01
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. — CodeCaster, Dec 05 '13 at 13:09
Do what @OndrejJanacek suggests. It's ___definitely___ the best way. — dav_i, Dec 05 '13 at 13:27
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... — NobodysNightmare, Dec 05 '13 at 13:34
@dav_i 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. — Manolo, Dec 06 '13 at 08:22
@NobodysNightmare 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. — Manolo, Dec 06 '13 at 08:24

Ondrej Janacek · Answer 1 · 2013-12-05T14:14:15.567

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.

edited Dec 05 '13 at 14:14

answered Dec 05 '13 at 13:24

Ondrej Janacek

12,486
14
59
93

2

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. – Daniel Mann Dec 05 '13 at 13:48
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. – Daniel Mann Dec 05 '13 at 14:05
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`. – Ondrej Janacek Dec 05 '13 at 14:07
1

It's your problem inasmuch as the answer you gave doesn't actually answer the question. – Daniel Mann Dec 05 '13 at 14:10
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. – Ondrej Janacek Dec 05 '13 at 14:12
Thanks for the input. As pointed out this does not solve the example given: calling Foo with two strings. – Manolo Dec 06 '13 at 08:01
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. – Ondrej Janacek Dec 06 '13 at 08:04
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. – Manolo Dec 06 '13 at 08:41

Daniel Mann · Answer 2 · 2013-12-06T14:11:29.130

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"});

edited Dec 06 '13 at 14:11

answered Dec 05 '13 at 13:52

Daniel Mann

57,011
13
100
120

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.) – Manolo Dec 06 '13 at 08:01
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 usage – Jun Wei Lee Dec 06 '13 at 14:17
@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. – Manolo Dec 09 '13 at 08:01

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

2 Answers2