Efcore: NotImplementedException is used in wrong meaning in EFCore DbSet class

[jinek]

Methods, of abstract class DbSet are throwing NotImplementedException, which means that this code needs to be rewritten/implemented: https://docs.microsoft.com/en-us/dotnet/api/system.notimplementedexception?view=netcore-3.1

Wrong usage of NotImplementedException leads to wrong handling of exceptions in user-level code.

More appropriate exception type can be used instead: NotSupportedException, InvalidOperationException etc. Or, probably, new exception type can be implemented for DbSet purposes.

[ajcvickers]

@jinek Can you elaborate on "Wrong usage of NotImplementedException leads to wrong handling of exceptions in user-level code?"

[jinek]

@ajcvickers In general, we can consider situation of throwing SqlException or NullReferenceException instead of this NotImplementedException. It would lead to similar, but, probably, bigger problems.

Some exception types, like OutOfMemoryException,CommunicationException, SqlException or in our case NotImplementedException has special meaning, which can be used to handle an exception in a special way.

For example:

switch(exception)
{
case CommunicationException: MessageBox("Can not reach the server");break;
case OutOfMemoryException: MessageBox("Not enough memory, please, try to close some applications"); LogForDeveloper(exception); break;
case SqlException: MessageBox("We have an error, please, restart the application"); LogForAdministrator(exception); break;
case **NotImplementedException**:MessageBox("We have an error, please, AVOID USING REQUESTED FEATURE until we roll out next release"); LogForDeveloper(exception) break;
}

If we mix exceptions, we have less possibilities to rely on exception type.

[ajcvickers]

@jinek Thanks. We'll follow up on the expected usages of NotImplementedException.

[ajcvickers]

Note for team: From https://docs.microsoft.com/en-us/dotnet/api/system.notimplementedexception?view=netcore-3.1

You might choose to throw a NotImplementedException exception in properties or methods in your own types when the that member is still in development and will only later be implemented in production code. In other words, a NotImplementedException exception should be synonymous with "still in development."

[roji]

FWIW in the same vein, some IDEs mark NotImplementedException in the same way as TODO.