You can’t make an omelet…

Article
01/18/2005

Sorry about the lack of updates. I haven’t been able to work on the Managed Language Service like I wanted to. Instead I went on a great vacation and then have been spending most of my time doing a lot of in depth security work on the C# language service. I’m not sure if I can talk about what we’ve done, but I’ll try to find out so I can write a post (or few) on that later.

So we’ve run into an interesting situation and we’re trying to figure out the best plan of attack for Beta2 (and for VS2005 in general). Best to dive right into specifics. In VS2003 we had a feature called “auto-complete on override” (AoO). The way it worked was that if you typed:

override<space>

then we’d pop up a list of all methods, properties and indexers that were available to be overridden. After choosing the item you wanted we’d then spit out code like:

public override int GetHashCode()

{

return base.GetHashCode();

}

Now, we received a lot of requests from customers who used features like this and “implement interface” and “+= on events” that they wanted more control over the code that was injected into the source. In order accommodate that, and also to enable some new features we wanted to provide, we developed the “expansions” feature that you can see in VS2005. These “expansions” use special user-editable .snippet files that can contain pretty much anything. They’ve got an XML schema that defines them and they feature specialized capabilities like the ability have programmatically replaceable fields.

So now we started using these “snippets” extensively and replaced all the places where we spit out a method to now use the snippet instead. To give you an idea of what that looks like, here’s the content of the MethodStub.snippet file:

<?xml version="1.0" encoding="utf-8"?>

<Title>Method Stub - Body</Title>

<Description>Snippet for generating method stub with a body</Description>

<Author>Microsoft Corporation</Author>

<SnippetType>Refactoring</SnippetType>

</SnippetTypes>

</Header>

<ID>signature</ID>

<Default>signature</Default>

</Literal>

<ID>Exception</ID>

<Function>SimpleTypeName(global::System.Exception)</Function>

</Literal>

</Declarations>

<![CDATA[$signature$

{

$end$throw new $Exception$("The method or operation is not implemented.");

}]]>

</Code>

</Snippet>

</CodeSnippet>

</CodeSnippets>

There’s a lot to digest, but really the most important components are the “literals” and the section inside the “code” tags. Inside that you’ll see the $signature$” field. This field will get replaced by the language service with the signature of the method being overridden. Next is the $end$ field. This designates where the cursor will be placed after the expansion is inserted. Finally there’s the $Exception$” field. This one is a little different. If you look at:

<ID>Exception</ID>

<Function>SimpleTypeName(global::System.Exception)</Function>

</Literal>

you’ll see that the $Exception$ field witll get expanded out into a function call to SimpleTypeName(global::System.Exception). The way that SimpleTypeName works is that it will use the “usings” in scope to pick the best possible name for the typename specified as its argument. That way if you have a “using System;” then we’ll inject “throw new Exception(…)” whereas if you don’t have that “using” then we’ll inject “throw new System.Exception(…)”.

Remember, this file is user editable. So by having this expansion, the user now has the ability to configure much of the code that is spit out. They can now add comments. Throw their own exceptions. Have a default doc-comment included by default. Etc.

So what does this mean? Well, between VS2003 and VS2005 we’ve changed the behavior of the AoO so that instead of getting:

public override int GetHashCode()

{

return base.GetHashCode();

}

you’d get:

public override int GetHashCode()

{

throw new Exception(…);

}

What’s worse is that there is no way to get back to the original behavior. i.e. you could change the snippet file to not throw the exception, but there’s no way to configure it to call the base method correctly (as that would require know far more information than is exposed to the snippet engine).

We received quite a few complaints about this internally and from ProductFeedback: https://lab.msdn.microsoft.com/productfeedback/viewfeedback.aspx?feedbackid=f0799869-79f0-4aa8-91d9-b2250dbc6a91

At first we figured that we would postpone adding this functionality back in a later release. We felt that while it was unfortunate that we were breaking old behavior it just wasn’t that big a deal. After all, a user could just manually make the call to “base” themselves. However, just recently we realized that our change was actually fairly detrimental to the user. Why? Well, for the correct functioning of overridable methods it’s often the case that the code must call the base method. Forgetting to do this can lead to subtle bugs where the consistency of the base object is not longer valid. By always injecting the base call we were putting that fact right in the face of the programmer and allowing them to remove it if they knew it was the right thing to do.

So we have a dilemma (albeit a minor one) right now. We can continue to ship the way the product works today. Do nothing and tell users “sorry, you can’t get the old behavior and you’ll have to manually fix up overrides.” Or suck it up and add back this functionality in a way that works with the current snippet architecture. i.e. we would most likely introduce a snippet that would have contents like this:

<![CDATA[$signature$

{

$end$$CallBase$”

throw new $Exception$("The method or operation is not implemented.");

}]]>

(note the addition of $CallBase$). This field would then expand to the relevant call to the base method, property or indexer.

Why wouldn’t we do the latter? Well, we’re focusing on security right now and absolutely getting VS2005 up to extremely high quality for the beta2 release. This is the VS released under the “go live” license, meaning you can develop web sites and software that you are allowed to release/sell/whatever. So it’s paramount that these tools be working very well. Toward that end we are fixing the bugs we consider important and, in general, postponing anything else unless we feel that there is enough user need to warrant their inclusion.

So what we’re trying to find out is how important this feature is to you. To you feel that it’s important enough to be re-added very late in the cycle (with the possibility of delaying beta2!), or should it be left out?

Let us know! Tell us here (or on ProductFeedback) so that we’ll know what the right thing to do is!

Thanks!

Comments

Anonymous
January 18, 2005
As I've indicated at http://lab.msdn.microsoft.com/productfeedback/ViewFeedback.aspx?FeedbackID=FDBK19467

I prefer TODO: task-list item over global search for NotImplementedException.

Anyway - with or without throwing exception statement - TODO: comments will be pretty nice to have in all autogenerated code. I like the way VC6 wizards put thouse comments at all correct places - unfortunately - VS 2005 does not do this and you must guess that to do next with your code.
In case if code generation rules will be hardcored somethere - you will be unable to fix this flaw without Microsoft assistance :-((

BTW, I've discusses this with Cyrus - and proposed to add one more variable $ReturnBase$ thich will be equal to "return $CallBase$" for all return types except "void" one.
This will allow developers to not add/delete return statements manualy to get compilable code for void/non-void method if throw statement will not be used.
What do you think ?
Anonymous
January 18, 2005
I don't think there is a 'right' way to do it. It depends on how the developer uses the 'AoO': to set up a skeleton for the class to be implemented later (an exception is best for this scenario), or to set up a skeleton for the method to be implemented immediately (the base call is best). I know I use it in both ways.

Most of the time I used the 'AoO' in VS2003, I immediately replaced the call to the base method with a 'throw NotImplementedException()', so I was very happy to see VS2005 do that by default.

View this from the scenario where you set up a skeleton for a derived class, but don't have the time to actually implement that new class yet (for instance you want to reference that new class in the code you are actually working on). Putting in NotImplementedExceptions is much safer than calls to the base implementation: your code can compile correctly, but attempting to run it will immediately warn you that you forgot to implement something (and IIRC FxCop reminds you as well).

On the other hand, having the call to the base implementation injected in your code saves a bit of typing later on and, as you already mentioned, reminds you that you may need to insert that call.

Of course, adding both the exception and the call will result in warnings about unreachable code (and I normally treat warnings as errors). How about inserting both, but commenting out the call?
Anonymous
January 18, 2005
P.S> BTW, I've notImpl expansion for this exception (no kidding !).
If I decide to postpone some activities in middle of coding - I use it - "notImpl<TAB>" and done !!

Here it is (notImpl.xml):
<?xml version="1.0" encoding="utf-8" ?>
<CodeSnippet Format="1.0.0">
<Header>
<Title>NotImplemented exception</Title>
<Shortcut>notImpl</Shortcut>
<Description>Expansion snippet for "throw new System.NotImplementedException();" statement</Description>
<SnippetTypes>
<SnippetType>Expansion</SnippetType>
</SnippetTypes>
</Header>
<Snippet>
<Declarations>
</Declarations>
<Code Language="csharp" Format="CData"><![CDATA[// TODO: XXX Fixme
throw new System.NotImplementedException();
]]>
</Code>
</Snippet>
</CodeSnippet>
Anonymous
January 18, 2005
AT:
I prefer TODO: task-list item over global search for NotImplementedException.

---

You're free to modify the snippet file to do exactly that.

Just get rid of the exception and add the //TODO: whatever
and you'll be all set.
Anonymous
January 18, 2005
Luc:
Of course, adding both the exception and the call will result in warnings about unreachable code (and I normally treat warnings as errors). How about inserting both, but commenting out the call?

---

You won't get a warning. Take the above example and you'll get:

public override int GetHashCode()
{
....base.GetHashCode();
....throw new Exception(…);
}

So there won't be any complaints about unreachable code.

This enforced the idea that you need to call the base method (but not necessarily return the result from it), and it also inject a good exception so that your unit tests will fail.
Anonymous
January 18, 2005
AT: i like the snippet :)

Maybe we'll add that to VS2k5!
Anonymous
January 18, 2005
"You won't get a warning."

Ah, Ok.

I missed the fact that the 'return' was missing. I'm using VS2003 as my everyday development environment, so my mind must have mentally inserted that 'return' that VS2003 would insert there...
Anonymous
January 18, 2005
I think I'd like most to have Beta2 on time than a feature like this, that you can retrofit in a second moment. But I like very much the $ReturnBase$ equal to "return $CallBase$" for all return types except "void" one.

I know that this could be a little more complicates, but how do you think of making the method more flexible? maybe making it a function call like SimpleTypeName(global::System.Exception)? I mean, you could have CallFunction($MethodName$) and write base.$callfunction$, or even this.$callfunction$ or proxytarge.$callfunction$. Or return FilterMethod($callfunction$).
In this way you can also define $ReturnBase$ in terms of CallFunction.
Anonymous
January 18, 2005
If you don' thave time to implement $CodeBase$ why not have the default snippet atleast make a suggestion to the user that he or she might which to call the base class?

//Todo: call base function if necessary
Throw new Exception(...)

Atleast ur putting the fact in the face of the programmer, somwhat...

Karl
Anonymous
January 18, 2005
I think that shipping as is would be a disaster in the making. It seems to me that there are a lot of devs who wouldn't understand how to make it right. For the sake of the junior engineer out there in the world, I would say make the change.
Anonymous
January 18, 2005
AoO is a great feature.

Expanding it to snippets is nice, but the important thing to me is what it does now.

It gives a result specific to the override you choose. For those who always want the same thing (NotImplementedException) I suggest copy-paste.

If you have the method's signature,return type and parameters, why can't you determine what a call would look like?
Anonymous
January 18, 2005
The comment has been removed
Anonymous
January 18, 2005
IF it's going to delay Beta 2, then DON'T DO IT, I can live with this "problem"... but I CANNOT live without beta2...!!!!
Anonymous
January 18, 2005
I would like to see snippets in Beta 2... Giving the user this flexibility with the product is a good thing...
Anonymous
January 18, 2005
The comment has been removed
Anonymous
January 18, 2005
By the way... I didn't see it as an option... but, if you ask me going back to VS 2003 style is better than the current.

JavaKid.
Anonymous
January 19, 2005
The comment has been removed
Anonymous
January 19, 2005
By the way how do you manage inserting properties with snippets? Do you have three different snippets for "getter only", "setter only" and "both"?
Anonymous
January 19, 2005
I consider this a low priority. If the developer is overriding a method on a base class, they really should know whether or not they need call the base implementation. The developer should make that decision, not the IDE.
Anonymous
January 19, 2005
Please make the fix. I agree with Jim Argeropoulos's well-phrased comment.
Anonymous
January 19, 2005
Karl: I like the suggestion about the TODO :)
Anonymous
January 19, 2005
Andrew: "If you have the method's signature,return type and parameters, why can't you determine what a call would look like? "

Yes, it's not hard problem to do this, it's just a question of whther or not it's worth the cost.
Anonymous
January 19, 2005
Jim: "I think that shipping as is would be a disaster in the making. It seems to me that there are a lot of devs who wouldn't understand how to make it right. For the sake of the junior engineer out there in the world, I would say make the change. "

Very good point. I'll let everyone here know about that!
Anonymous
January 19, 2005
The comment has been removed
Anonymous
January 19, 2005
Stuart: Great suggestion on $start$ ... $end$

I'll see if i can get taht done as well!
Anonymous
January 19, 2005
"This started as a C# feature, but ended up moving to all languages."

That is both great and a real pitty - great for other language users, a pity for us C#'ers that it reduced the dev time for neat features.

Still, having played with VS2k5 now (I'm in love!) I'd say that having the ability to call a base class is of significant value to me.

Should it delay beta2? Frankly I don't care, but it should be in the final release...

I also just noticeda bug in snippet expansion - probably it has already been reported - but if I use a snippet expansion for a 'for loop' and then try and repeat this inside that loop to generate a nested set of loops it fails.

for(int i = 0; i < iMax; ++i)
{
for//Placing cursor here and trying to expand snippet does nothing.
}

cheers
Anonymous
January 20, 2005
>> Should it delay beta2? Frankly I don't care, but it should be in the final release...

+1
Anonymous
January 20, 2005
Postpone BETA 2 and make sure you don't break any editor behaviour! There are just too much such small discrepancies between VS.NET 2003 and VS.NET 2005. It will make switching to VS.NET 2005 a pain, which is a pity, because all the great features won't counter-balance it, from my point of view.
Anonymous
January 20, 2005
It needs to work the way it used to work.
Anonymous
January 20, 2005
"BTW, I've discusses this with Cyrus - and proposed to add one more variable $ReturnBase$ thich will be equal to "return $CallBase$" for all return types except "void" one. "

Or you could support void returns so that you can actually do things consistently....
Anonymous
January 20, 2005
I'd like it put back, and I'd like it with the "return" - but mostly, for my money, I'd like to see the throwing of the exception appear above the base call, just to enforce that I look at all those overrides before calling them...
Anonymous
January 21, 2005
The comment has been removed
Anonymous
January 21, 2005
Joku,

Actualy - link to documentation will become very annoying after 2 day of usage.
Even more - my "// TODO" comments idea in notImpl sniplet must be changed a little bit to become usefull.

Instead of always writing the same TODO comment - you must set user cursor at location there he can write his own personal comments about reasons why this is not implemented yet.

While this is possible to automaticaly generate some predefined comment - this comments text must be selected to make it possible delete/override with a single keystroke / letter.
Anonymous
January 22, 2005
AT,

Perhaps, anyway that is the simplest way to add a note about the issue (atleast until it is fixed, if that is still possible before final) and still making the beta2 deadline.. Unless someone invents something better :)
Anonymous
September 02, 2005
Super site!
Anonymous
May 29, 2009
PingBack from http://paidsurveyshub.info/story.php?title=cyrus-blather-you-can-t-make-an-omelet
Anonymous
June 16, 2009
PingBack from http://workfromhomecareer.info/story.php?id=21121
Anonymous
June 16, 2009
PingBack from http://topalternativedating.info/story.php?id=15615

Share via

You can’t make an omelet…

Comments

Additional resources