Evil code of the day

re: Evil code of the day

Yelinna — Tue, 11 Aug 2009 20:49:37 GMT

Happily, this is an unlikely situation. Why would you want to overload a method with another method with the same name, the same number of patameters and their same types?

Overloading is not for this coding evilness!!!

re: Evil code of the day

Leo Bushkin — Thu, 16 Jul 2009 18:44:57 GMT

Minimally, the compiler should provide a warning when arguments to a method are renamed in an overriding method, or reordered at a call site.

re: Evil code of the day

Leo Bushkin — Thu, 16 Jul 2009 18:39:29 GMT

This is something that really bothers me about the optional parameter spec in C# 4.0 - that and the fact that optional values are actually compiled into the callsite, rather than dynamically fetched at runtime from the assembly's metadata and then cached.

C# 4.0 has essentially made the parameter names a weak part of the signature of the method - as far as overload resolution is concerned for optional/named parameters.

I have to say, that I believe that this should be a case where the compiler detects and prohibits the override in the derived class.

In fact, the compiler should enforce that when the override keyword is applied to a method, the argument names must match the base method in both order and name.

It may not be perfect, especially when parameter names can change in the base class over time, or conflict with other overloads - but it's better than changing the overload resolution on the fly based on which version of the framework you compile to - or whether you call the method through a base or derived reference, or through a dynamic reference.

re: Evil code of the day

Trumpi — Sun, 12 Jul 2009 11:55:50 GMT

I hope that there is an FxCop rule for this!!

re: Evil code of the day

ShuggyCoUk — Tue, 07 Jul 2009 19:13:39 GMT

I added my +1 to the connect feature

re: Evil code of the day

Koush — Mon, 06 Jul 2009 15:54:59 GMT

It's more evil that the parameters on an overridden method were renamed. ;)

re: Evil code of the day

runefs — Mon, 06 Jul 2009 08:07:30 GMT

Rather unfortunate that the specifications basicallty breaks Liskov.

If you for some reason renamed the parameters of one of your classes you can no longer make the substitution with out breaking your code.

You might argue that the the developer is breaking the principle by using named parameters (and changing names) but I none the less finds it questionable when the language makes that possible

re: Evil code of the day

skeet — Sun, 05 Jul 2009 20:26:52 GMT

@David: Admittedly I hadn't checked whether VB had named arguments. (Even C# has had named *parameters* forever :) Yes, I hope it's mentioned in the FDG. I suspect it's a guideline which hasn't been treated particularly seriously in the past though - there's nothing like the risk of it happening *within your own language* to highlight a potential problem to developers :)

(Of course, it means that it's yet more names to try to get right - and we know what a pain *they* are.)

re: Evil code of the day

David Nelson — Sun, 05 Jul 2009 20:18:24 GMT

Anyone who has been concerned with interoperability has always considered parameter names to be a part of the public API, given that VB has had named parameters from the beginning. I believe there is a statement about that in the Framework Design Guidelines, although I can't remember where at the moment.

re: Evil code of the day

skeet — Sun, 05 Jul 2009 11:50:30 GMT

@Joren: Yes, it's certainly my code which is evil here. I mainly wanted to draw attention to the fact that something which was previously harmless (renaming parameters) can now be very harmful indeed - and if you just reorder parameter names (instead of giving them completely different names) you can end up causing a bug quite easily.

Likewise changing the declared type of a variable now not only affects which overloads are considered, but also which argument means what.

I'm in no way trying to say that named arguments are a misfeature: just that they also introduce more opportunities for error. Use with care, and be aware that parameter names are now part of the API.

Jon

re: Evil code of the day

Joren — Sun, 05 Jul 2009 11:11:06 GMT

There is nothing nasty about the language here. Only the derived code is nasty.

You can't use the parameter names for Base.Foo when directly calling Derived.Foo. If you have a method with parameters 'foo' and 'bar' it's ridiculous to have to call it with 'blah' and 'zing' just because those happen to be the parameter names in some ancestor of the method. Just think of the confusion this would cause.

You also can't use the parameter names for Derived.Foo when calling Base.Foo, because there is no reason to suppose that Derived.Foo is any better for taking the names of than SomeOtherDerived.Foo. In fact, there might not even be any derived classes.

So you always have to use the parameter names for the method you're calling, regardless of it being virtual or an override or not. Which makes perfect sense.

There is nothing else the language could logically do. It's your code that is evil. And there's a simple solution for that: don't write this code (and who would!) :-)

PS. I don't actually see you saying that the problem is in C# and not your code per se ... in fact I basically see you saying the opposite. But then, why call attention to the obvious fact that even if there's nothing wrong with a language you can still do horrible things in it? I don't understand.

re: Evil code of the day

Jeroen Landheer — Sat, 04 Jul 2009 21:59:38 GMT

Code analisys (FxCop) has a rule for this one: "Parameter names should match base declaration" (CA1725)

This rule only checks types with a public access modifier though.

re: Evil code of the day

Eamon Nerbonne — Sat, 04 Jul 2009 21:23:56 GMT

C# devs have historically seemed reluctant to add new warnings, the motivation apparently being that this may break users with "Warnings as Errors" on. For instance, even obviously unsafe usage of IDisposable classes still doesn't cause a compiler warning (when it really should).

So, if that's really a prime motivation, then adding a warning at the call site may be lower risk to them - after all, you can't break old code that way, since no old code has named parameters _anyhow_.

re: Evil code of the day

skeet — Sat, 04 Jul 2009 20:39:27 GMT

@James: At that point the dynamic version and the statically typed version would presumably diverge in behaviour.

Let's not do that :)

For what it's worth, I'm going to make it very clear in C# in Depth that named arguments make parameter names effectively part of the API for public members: change them at your peril!

re: Evil code of the day

James Hart — Sat, 04 Jul 2009 20:35:52 GMT

For extra giggles, I find myself wondering what happens if Base, Derived, and PureEvil all live in separate assemblies, and you fiddle with the parameter names in Base and/or Derived without recompiling the other assemblies...

re: Evil code of the day

skeet — Sat, 04 Jul 2009 18:57:37 GMT

Right, Connect suggestion added at connect.microsoft.com/.../ViewFeedback.aspx

It'll be interesting to see what the feedback is like :)

Jon

re: Evil code of the day

skeet — Sat, 04 Jul 2009 18:40:23 GMT

@ShuggyCoUk: That's a very interesting idea - I'd actually suggest putting the warning on both the call site and the overriding code.

@John: Parameter names are the names of variables, but in C# 4 you can specify named arguments, which can be in a different order to the declaration order.

re: Evil code of the day

John Kraft — Sat, 04 Jul 2009 14:59:24 GMT

I thought that the named parameters were only supposed to be 'labels' for the parameters, and not effect behavior at all???

re: Evil code of the day

ShuggyCoUk — Sat, 04 Jul 2009 14:56:50 GMT

I would suggest that they should add a compiler warning for any call site using named parameters where the parameter names involved have changed in the virtual hierarchy.

re: Evil code of the day

skeet — Sat, 04 Jul 2009 10:14:50 GMT

@Tom: I think C# 4 will help a little bit in many situations, and a lot in a few situations. If you're working with COM it'll be a lot nicer. If you're working in a C# 4-only shop, optional parameters can reduce the need for lots of overloads, and named arguments *can* add readability when used sensible. Covariance and contravariance just removes a little roadblock (in some situations). Dynamic typing is great for interop with COM or dynamic languages, and also makes reflection easier.

It's not *nearly* the same size of change as C# 2 or 3 though. And yes, I'd like to see some more const/immutable help too.