Implement Interface (Redux)


Sorry for the poor appearance of some of the code samples.  The new blog site seems to be having problems with the markup.  I’m working with them to try and find a solution ASAP.


Ok.  Until i find a solution, i’m just going to use screen captures.


 


—-


 


We’ve made some recent changes to the Implement Interface (II) feature and I wanted to go over them here to let you know about them and to explain our reasoning for the changes.  Hopefully you’ll agree with the choices we made and you’ll like the feature even more.  I’ve always had some gripes with how II worked in the past (and in beta1 and the beta2 build you’re going to receive) and I decided that these gripes would actually be things that would be unpleasant for the user and really needed to be fixed.  Specifically, the feature was not only confusing for the user in terms of the job it performed, but it was also pretty dumb when it came to doing that job.


 


(note: the same problems apply to Implement Abstract Class; but as that feature is basically identical to II I’m just going to refer to the entire issue by just dealing with II)


 Let’s take a look at II back in beta1:


              (Implement interface smart tag tickler)


             (Implement interface with tickler expanded) 


Ok, looks fairly good right?  Underneath an interface that a type is declared to implement you get options on how to make this type implement that interface.  So what’re the problems?  Well, first off, the choices are just plain confusing for many people.  Explicit interface implementation is not a feature that is often used (or even known) by many, and if you were to ask someone “what’s an implicit implementation?” they probably couldn’t tell you.  But, that’s not really a huge problem.  People will learn pretty quickly since they’ll probably try both options and will see how they differ.  So what’s the real problem for me?  Well, the fact that we’re basically lying to you when offering those options.  What do I mean by that?  Well consider the following code:


            


The problem, of course, is that it’s impossible to implement this interface implicitly.  If you tried you would get:


                         


which is illegal in C# (methods with the same parameters can’t have differing return types).  Instead what you need to write is:


            


i.e. explicitly implement any methods which are going to conflict with other members (either from the same interface, or which already exist in the type).  Note: Implement Abstract Class has a worse time since it has no way to explicitly implement members when there is a conflict.  A later post will discuss this pesky issue).


So why did we call the option: “Implement interface ‘IFoo2’ stubs Implicitly”?  Pretty much because in the beginning that’s all it did, and only when we ran into this issue did we realize that the option name was a rather poor one.  So what should the option be called?  We thought about: “Implement interface ‘IFoo2’ stubs Implicitly, unless there is a conflict, in which case implement the conflicting sub Explicitly”, but that seemed a bit too verbose and confusing, (and far too long for an onscreen option J), and so we settled on the following:


            


The first option now means: “implement the interface doing whatever necessary to make legal code”, i.e. implicitly when possible, but explicitly if not; whereas the second is for people who really want everything to be explicit. 


Ok, so this is a tiny bit better than before.  But I’m sure you’re saying: “you know, I really couldn’t care less about a minor wording change like that”, and really, if that was all we changed I’d agree that that wasn’t worth talking about.  However, since there was a whole bunch of work done here, I wanted to talk about all of it.  So now onto change #2:


Let’s look at another example:


            


Now, prior to my changes what you’d actually end up with was:


            


i.e. besides just implementing the members in IList, we’d also reimplement the ones in ICollection.  The logic was set up so that when you implemented an interface we would implement all of its methods, regardless of whether they’d already been implemented by a supertype.  As it turns out this is completely legal in C# and, of course, it made the implementation of the II feature quite simple.  However, whenever I used the feature this ended up being so aggravating since it was never what I wanted.  CyrusList already has an Add method (inherited from CyrusCollection) so why re-implement it, especially since all the user is going to do is hunt through the newly spit code and just delete these extraneous methods.


We struggled over this for a while.  Were there users who would prefer our model of spitting out everything?  After all, if you don’t like it you can always remove the extra junk spit in the code, but if you do want it then there is no way to get it.  But, in the end we decided that in the common case our default behavior would just end up annoying the user and so we went back to our definition of what “Implement Interface” meant and revised our definition to:


            “implement the interface doing the minimal amount of work necessary to make legal code”


We feel that when a user selects this first option what they are really saying is: “look, there is some grunt work that needs to be done so that I actually implement this interface and get into a compilable state.  Just do that work for me so I can get on with the actual interesting work of putting the right logic into those new members”.  And so doing the minimal work now fits in with providing them a helpful service without tripping them up.


There’s also one more thing we changed about the II feature.  Let’s go back to the original example.


            


You decide to implement the interface, and sweet, the interface is implemented for you!


              


Except… this is odd, there’s still a smart tag under the interface declaration.  What happens if i expand it?


            


Hunh… “implement interface implicitly” is still in the list.  Waitaminute… I just implemented that interface!  Why is that option still there?  Let me try using it… and it does nothing!  Sweet!  Offering an option that silently does nothing when invoked!! That’s always a popular feature in any application, and really is something that good designers tell you you should be doing.


Turns out there’s already a feature we have in VS2005 that has behavior that’s more in line with what we’d like to see here: Generate Method Stub (GMS).  If we see that there’s already an existing method that you could be calling, then we don’t offer to generate the stub for you.  This is, of course, completely necessary.  If we didn’t do this then under every method call you’d see this silly smart tag and you’d probably kill us for distracting you with all that clutter.  However, just because there we be less clutter with II (since there are less interfaces implemented than method calls in your code) doesn’t mean that it’s ok to keep offering you useless options.  So our final change to II was to make it so that if it has no work to do then it doesn’t both even offering the option to you.  The experience is now a lot nicer for you and fits the GMS model where we offer the option and then don’t show it after you’ve used it.


            


Now, this was somewhat of a large functionality change to make late in the game, and so I wanted to do it in the simplest way possible (i.e. DoTheSimplestThingThatCouldPossiblyWork). So what I did was look at the logic that we currently have for II and I broke it into two parts.  The first part does all the work walking interface lists and determining which members need to be implemented and it also generates all the code that needs to be spit back into the user’s code.  i.e. it’s all the heavy lifting.  All the second part does is literally call ReplaceLines on the user’s buffer with the code generated by the first part.  After refactoring the code in that way, all I did was make it so that when we query to determine if the II option is available we do the entire first step.  Then, if we see that that produced no text to spit into the code then we respond by saying “no, this option is not available”.  This refactoring allowed me to make an extremely small code change that didn’t actually affect any of the core logic of II (which I was worried about touching and possibly screwing up), and left us with the very desirable model of: “if we know we would generate nothing then don’t even bother offering the option.”  Some people see that as being a fugly change.  “Why go through all that work, especially generating the code, if you’re not even going to use it?  What a waste when you could be doing this so much more efficiently.”  But, because of making this code change in this way, we were able to make the feature change with high confidence that it would not be a problem and we would not be regressing any behavior very late in the game. 


I hope you like the changes that we’ve made here.  Unfortunately you won’t see them until past beta2, but since I think that you’re going to be using this feature a lot I thought it would be worth it to spend that little extra effort on it to improve the entire user experience around it.


Let me know what you think!


Comments (19)

  1. DavidMKean says:

    How come the stubs now throw an Exception, instead of a NotImplementedException? I’ve written FxCop rules around the NotImplementedException so that I can see what methods are still needed to be implemented.

    By the way its hard to know how to post a comment; I found that you needed to create a new account and log in to do so.

  2. How about a feature that lets you convert an existing implicitly implemented method into an explicit one (and vice versa). Sometimes (especially with regard to collections) I want most of the methods implemented implicitly, but a few implemented explicitly.

    A smart-tag for this would be very nice…it would not even have to fix the code that calls the method (in the case of implicit-to-explicit conversion), although that would be a great added bonus.

  3. CyrusN says:

    David: "How come the stubs now throw an Exception, instead of a NotImplementedException? I’ve written FxCop rules around the NotImplementedException so that I can see what methods are still needed to be implemented. "

    This is somewhat of an unfortunate situation. The compact framework 1.0 doesn’t have the NotImplementedException and we don’t want to generate code that will break in that situation.

    For now you can go in and change the snippet that generates the code to throw NotImplementedException.

    We’re trying to figure out something better for RTM.

    "By the way its hard to know how to post a comment; I found that you needed to create a new account and log in to do so. "

    THat’s been fixed now 🙂

    Sorry about that. We just moved to this new system and there were some minor glitches.

  4. CyrusN says:

    Muuj: "How about a feature that lets you convert an existing implicitly implemented method into an explicit one (and vice versa). Sometimes (especially with regard to collections) I want most of the methods implemented implicitly, but a few implemented explicitly. "

    Sounds like a nice feature. We could definitely add taht in the future. If you really wanted it "now" you could write a tool to do it based on teh codemodel.

    "A smart-tag for this would be very nice…it would not even have to fix the code that calls the method (in the case of implicit-to-explicit conversion), although that would be a great added bonus. "

    It would have to fix up calls in that case. Imagine the following:

    class Foo : IDisposable

    {

    ….public void Dispose() {}

    ….void Bar()

    ….{

    …….this.Dispose();

    ….}

    }

    if you change Dispose to be explicit, then you need to fix up the call like so:

    class Foo : IDisposable

    {

    ….void IDisposable.Dispose() {}

    ….void Bar()

    ….{

    …….((IDisposable)this).Dispose();

    ….}

    }

  5. Kevin Dente says:

    Sounds like a good change. I’m in favor.

    The missing interface-related intellisense feature that’s killing me is the inability to easily implement a single interface method. Doing TDD (especially model-view-presenter UI development), I tend to incrementally build up interfaces, and there isn’t a good way to implement just the a newly added interface method (re-selecting "implement" from the smart tag is close, but it generates extra regions).

    There was actually a way to do this in VS2003, so 2005 is a step backward here. It’s driving me bonkers. If only the "override" intellisense would list interface methods instead of just virtual base class methods, it would do exactly what I need.

  6. CyrusN says:

    Kevin:

    "The missing interface-related intellisense feature that’s killing me is the inability to easily implement a single interface method. Doing TDD (especially model-view-presenter UI development), I tend to incrementally build up interfaces, and there isn’t a good way to implement just the a newly added interface method (re-selecting "implement" from the smart tag is close, but it generates extra regions). "

    We have a reg key that allows you to not spit out the region tags. I spent some time seeing if i could make it so that it would spit into the proper regions, but found that it was too difficult given the current architecture.

    "There was actually a way to do this in VS2003, so 2005 is a step backward here. It’s driving me bonkers. If only the "override" intellisense would list interface methods instead of just virtual base class methods, it would do exactly what I need"

    How did you do it in VS2003?

  7. All screenshots are gone?

  8. Kevin Dente says:

    Cyrus,

    In VS2003 you could use ClassView to implement a single interface member. You navigate to your class, expand bases and interfaces, expand the interface, right-click on the method, and select Add/Override. That option has been removed in 2005.

    It wasn’t intellisense driven, and took too much mouse clicking, but at least it was something. Now we don’t even have that.

    The reg key to stop spitting out regions sound promising. What’s the key?

  9. Thomas Eyde says:

    This must be a language bug?

    class Foo : IDisposable

    {

    ….void IDisposable.Dispose() {}

    ….void Bar()

    ….{

    …….((IDisposable)this).Dispose();

    ….}

    }

    I have to cast to call a method implemented in the same class? You got to be kidding?

    Why not do the simplest thing? Call it as you would call any other method. Bar() is one name, IDisposable.Dispose() is another:

    ….void Bar()

    ….{

    …….IDisposable.Dispose();

    ….}

  10. Thomas Eyde says:

    I was surprised that IFoo2 : IFoo1 was compilable. Shouldn’t it be possible to move a member from one A to B when B : A?

    Oh, I think I see it: It is that implicit new keyword, isn’t it? IFoo2 actually hides the member in IFoo1. I don’t like that feature, it’s very confusing. It’s basicly saying that you have an instance of this type, but sometimes you don’t.

  11. CyrusN says:

    Kevin: "The reg key to stop spitting out regions sound promising. What’s the key? "

    Currently it’s:

    HKCUSoftwareMicrosoftVisualStudio8.0CSharpEditorImplementInterface_InsertRegionTags

    you need to create that key and set teh DWORD value to 0.

    I’m looking into exposing that from the UI if possible.

  12. CyrusN says:

    Thomas: "This must be a language bug?"

    No, that’s exaclty the purpose of explicit interface implementation. In order ot deal with conflicts between interfaces we allow you to explicitly tell which interface’s method you are implementing.

    Then to call the method you need to call through the appropriate interface so as to disambiguate it.

    Imagine if you had:

    class Foo : IDisposable, IDisposable2

    {

    ….void IDisposable.Dispose() {}

    ….void IDisposable2.Dispose() {}

    ….void Bar()

    ….{

    ……..this.Dispose();

    ….}

    }

    which dispose are you calling?

    "IDisposable.Dispose() is another:"

    That’s doesn’t work either. What if you have a local/field/property with that name. WHy should is suddenly bind to teh interface instead?

  13. CyrusN says:

    Thomas: " was surprised that IFoo2 : IFoo1 was compilable. Shouldn’t it be possible to move a member from one A to B when B : A? "

    I’m surprised that you’re surprised 🙂

    And yes, you would get warned in the above cases about missing hte "new". I left it out because it didn’t matter either way for the topic at hand.

    "I don’t like that feature, it’s very confusing. It’s basicly saying that you have an instance of this type, but sometimes you don’t. "

    Could you explain what you mean here? when is the "sometimes you don’t. "??

  14. I’m sorry. I guess what I *meant* to say was that I would not care if it did not fix up the calls. I would probably only use the conversion feature when designing classes initially, so it wouldn’t need to be a 100% refactor, just a nice timesaver.

    But like I said, the complete version would be great.

  15. CyrusN says:

    TheMuuj: "I’m sorry. I guess what I *meant* to say was that I would not care if it did not fix up the calls. I would probably only use the conversion feature when designing classes initially, so it wouldn’t need to be a 100% refactor, just a nice timesaver.

    But like I said, the complete version would be great. "

    We’ll definitely consider that in the future Muuj! However, i do recommend looking into the code model so that you could write this feature for yourself today 🙂

  16. Thomas Eyde says:

    When conflicting members are allowed, you are forcing me to know how the interface is implemented. When I look at

    class Fooey : IFoo2

    How should I know there is an IFoo1 in there? Let me do this different:

    IBigFoo : IFoo1, IFoo2

    Then I consider

    class Fooey : IFoo1, IFoo2

    to be different from

    class Fooey : IBigFoo

    That’s why I was surprised this code compiles. Which leads me to:

    When one type hide a member of another type it is, if not lying, at least deceiving. If I have something like

    Animal animal = new Cat();

    then I expect animal to behave like a cat. That is what the code communicates. But if Cat.Speak hides Animal.Speak, then the following produces different results:

    Animal a1 = new Cat();

    Cat a2 = new Cat();

    a1.Speak();

    a2.Speak();

    The code is technically correct, but it’s confusing. I have a Cat, but sometimes I don’t.

    (Why do I have to scroll when I click "Post comment"?)

  17. duncan says:

    Is there any way of expanding smart tags without using the mouse?

    I’ve been playing with Beta 1 and having to reach for the mouse every few characters is driving me nuts… I’m sure in 2K3 I could just hit tab to implement an interface which was much more usable if less flexible.

    Could we have that feature back? Even if it meant having to hit <tab> twice to get the default option it would be nice…

  18. CyrusN says:

    Duncan: "Is there any way of expanding smart tags without using the mouse? "

    Yes. The shortcut should be: ctrl-dot

    (although i don’t know if that made it into beta1).

    In the worst case it’s something like: alt-shift-f10

    (this is, of course, configurable in the keyboard shortcuts options).

    Send me a message if this doens’t work for you.

    (i’m not in front of my comp so i can’t verify).

Skip to main content