What's wrong with bodyless abstract methods in abstract class?

Question

I'm refactoring a pre-existing solution. I use ReSharper and I've noticed a code inspection rule is being tripped. There is an abstract class which has bodyless method signatures with the intention of forcing the derived classes (of which there are several). As far as my knowledge goes, this is the (or at least a) correct way to do things. However, ReSharper is complaining that the "Type member is never accessed via base type" and that "Only overrides of [the methods] are used." Here is example code that replicates the issue in question:

public abstract class MyAbstractClass
{
    public abstract void CreateSomething();
    public abstract void ReadSomething();
    public abstract void InsertSomething();
}

public class MyDerivedClass : MyAbstractClass
{

    public override void CreateSomething()
    {
        throw new NotImplementedException();
    }

    public override void ReadSomething()
    {
        throw new NotImplementedException();
    }

    public override void InsertSomething()
    {
        throw new NotImplementedException();
    }
}

By the way, there are other members that rule out making the abstract class an interface. ReSharper suggests making changes to the 3 methods in the abstract class. Its suggestions are to make them protected, virtual, non-abstract or to simply remove them from the abstract class and only have them in derived classes. Whoever originally wrote this code intended for every derived class to implement these methods, and for those methods to be public in the derived classes. So, is there some way I should change this to make it more effective? If not, why is ReSharper taking issue with this?

Dignitary

Do you have 'Solution-wide analysis' turned on? Bottom right corner, green radar-kind-of-thingy. – Inapprehensible

@JeroenVannevel - Yes I do. – Dignitary

It's telling you that because it notices there is no other subclass in the solution that implements from the baseclass, essentially making it useless. It does surprise that it shows this warning with public classes -- are you sure they are? To verify my suggestion: add a second class to your solution and have it inherit from the abstract class. Does that remove the warning? – Inapprehensible

@JeroenVannevel - There are several classes which are implementing the abstract class. This is why I'm confused. The complaint is that I'm only implementing the methods in derived classes, but that was the intention. I don't want a default implementation. – Dignitary

@Dignitary It's because none of your code operates on MyAbstractClass. In this sense, yes, the abstract class is meaningless. Write a stub method which takes a MyAbstractClass object and invokes the methods on the object. This should make the warning go away. But in any case, you can safely ignore it (unless you get the warning at the end of development, then you may want to revisit the architecture if you're never actually using calling CreateSomething on a MyAbstractClass object. – Hemialgia

To add to what @Hemialgia has pointed out, where you use the derived class, you could have declared it as the base class. Some will argue you ought always use the least specific type possible when declaring a variable, and if you do, this issue will go away. – Farinaceous

@BrianSweeney Also a great point. This issue came up at an old job where the code base was slightly disorganized and filled with half-baked ideas from previous developers. It was when I was first using ReSharper. When I installed it, practically every character in the solution got underlined. ReSharper has taught me a lot since that day! – Dignitary

Answer 1

As mentioned by Rob in the comments, just use the abstract base class as type, while creating instances of the derived class in your code, e.g.

    MyAbstractClass myInstance = new MyDerivedClass(); // Polymorphism applied
    myInstance.CreateSomething();
    myInstance.InsertSomething();
    myInstance.ReadSomething();

instead of creating new instances like this

    var myInstance = new MyDerivedClass(); // Polymorphism NOT applied

or this

    MyDerivedClass myInstance = new MyDerivedClass(); // Polymorphism NOT applied

to consider and solve the R# hint:

By doing so, Polymorphism is applied correctly in the code, which was certainly the intention of the creators/maintainers of your pre-existing solution.

Greg

@bubbleking: The comment of Rob was also helping me, to fix this ReSharper issue. I agree with you, that this comment shall be posted as an answer and therefor give one. Please have a look to it, if this answer is written in an understandable manner and helps. Thanks. – Greg

Answer 2

Because you are never accessing the method using a reference of type MyAbstractClass, there is no point in making it an abstract member--you could leave it out of the base class entirely and everything would compile just fine.

Bartender

The point I am reading from the previous developer's intentions is to ensure that all derived classes implement those methods. If left out of the base class, how is this possible? – Dignitary

@Dignitary It's likely a warning from resharper that as the code is now - you do not need to use an abstract class. Trying casting your object to MyAbstractClass and using the method. So instead of this: var something = new MyDerivedClass(); something.CreateSomething(); Use: MyAbstractClass something = new MyDerivedClass(); something.CreateSomething();. In any case, you can ignore the warning from resharper, as it's likely you will be referencing the methods through MyAbstractClass in the future (otherwise it is correct in saying the abstract method is not required). – Hemialgia

@Hemialgia - This is a fabulous explanation. Perhaps combine your comments into an answer? – Dignitary

I have accepted this answer, despite feeling that it is somewhat cursory, because it was made clearer in the comment by @Rob. – Dignitary