Bug 1357 - Add in NotImplementedExceptions in overridden methods
Summary: Add in NotImplementedExceptions in overridden methods
Status: RESOLVED FIXED
Alias: None
Product: Xamarin Studio
Classification: Desktop
Component: C# Binding ()
Version: 2.8
Hardware: Macintosh Mac OS
: Normal enhancement
Target Milestone: ---
Assignee: Mike Krüger
URL:
Depends on:
Blocks:
 
Reported: 2011-10-07 21:14 UTC by Chris Hardy [MSFT]
Modified: 2013-01-08 02:12 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Chris Hardy [MSFT] 2011-10-07 21:14:38 UTC
Is it possible to automaticly add the default not implemented exception like visual studio does for overridden methods, like:

public override int GetComponentCount(UIPickerView pickerView)
{
      //TODO: Implement - see: http://go-mono.com/doc/index.aspx?link=
      throw new NotImplementedException();
}

instead of the current implementation?
Comment 2 Mike Krüger 2011-10-10 02:10:09 UTC
I don't really understand what the bug is about - the exception is added ... why add it again ?

Or is the request about 'changing' the implementation of override methods  ?
Comment 3 Mike Krüger 2011-10-27 05:54:45 UTC
alan: I don't undestand why we add that TODO in the 1st place. But it was requested & should be ok.

Should we close that bug ?
Comment 4 Alan McGovern 2011-10-27 06:08:34 UTC
This is a feature of MonoTouch which probably needs better explaining. The comment seen there links to this page: http://docs.go-mono.com/index.aspx?link=T%3aMonoTouch.Foundation.ModelAttribute which explains how optional methods are treated in MonoTouch. This is not a bug per-se. I did question this comment before because it's a bit odd and I didn't really understand why it was there or what it was trying to tell me. I still don't really know why I'm being directed to that webpage to read the docs on the Model attribute ;)

Let me direct this towards mhutch who might be able to better explain the logic behind why this comment was added. Maybe we can improve it and/or the docs to make it more obvious why it's important to know what the Model attribute is used for and why it matters.
Comment 5 Mike Krüger 2011-11-15 15:12:58 UTC
Upping that to 'normal' ... we should explain it somehow better ... I've 'problems' with this behavior as well. It's likely that our customers won't understand it & will complain.

And even if they don't complain - they may have 'strange' feelings about this feature ...
Comment 6 Mikayla Hutchinson [MSFT] 2012-05-02 15:36:36 UTC
That was originally added because Miguel requested it, IIRC. Unfortunately the linked page doesn't really explain it in a user friendly way.

Normally, because the base method is virtual, MD would emit a base call when overriding. However, a virtual method on a Model class is special - its only purpose is to provide a signature that can be overridden. So, the base implementation throws an "don't call this" exception, and should not be called.

I think what the user is asking for is for MD to generate a "throw new NotImplementedException();" like it does for abstract members, instead of doing nothing. I agree with this, it would be more consistent. If we select the throw statement after generating it, then the user can immediately type or backspace to delete it.

 I'm not sure whether we should remove the comment, or replace it with something that's a bit clearer, e.g.

public override int GetComponentCount(UIPickerView pickerView)
{
    // NOTE: Don't call the base implementation on a Model class
    // see http://docs.xamarin.com/ios/tutorials/Events%2c_Protocols_and_Delegates 
    throw new NotImplementedException();
}

I'd be tempted to just remove it, it's annoying noise after the first time it's inserted, i.e.:

public override int GetComponentCount(UIPickerView pickerView)
{
    throw new NotImplementedException();
}


Miguel, thoughts?
Comment 7 Miguel de Icaza [MSFT] 2012-05-02 17:16:09 UTC
Hey guys,

Even better: if we lookup the base class and it is a class that has been flagged with the [Model] attribute, we can insert throw new NotImplementedException, otherwise we insert the "base" call or whatever Visual Studio does.
Comment 8 Mike Krüger 2013-01-08 01:48:09 UTC
Miguel: We do that - the thing is how the body should look like.
Comment 9 Mike Krüger 2013-01-08 02:12:17 UTC
For now I've choosen the verbose output, yes it's noise but it's selected (including the throw statement) and with a backspace/delete it all goes away.

The old comment was more distracting than helpful