Why would I want to always have to explicitly call a "base" method if I just want to use base functionality?

I recently worked with an architect who structured his base class like this:

public abstract class Base
{
    public abstract T Get(int id);

    internal T InternalGet(int id, IRepository repository)
    {
       // Do more magic
       return repository.Get(id);
    }
}

And then in the derived class, if you just want to use the base functionality, you'd have to..

public class Derived : Base
{
    private readonly IProductRepository _repository; // Implements IRepository

    // Ctor with dependency injection, this is good stuff!
    public DerivedClass(IProductRepository repository)
    {
       _repository = repository;
    }

    public override Product Get(int id)
    {
       // ... explicitly call base functionality - NOT a call to base though!
       return InternalGet(id, _repository);
    }
}

Why not just pass the repository to the base class, and override when you need to?

public abstract class Base
{
   private readonly IRepository _repository;

   protected Base(IRepository repository)
   {
      _repository = repository;
   }

   public virtual T Get(int id)
   {
      // Do magic
      return _repository.Get(id);
   }
}

And then in the derived class, if you don't want to use the base method, just override it.

public class Derived: Base
{
   private readonly IProductRepository _repo;

   public Derived(IProductRepository repo) : base(repo)
   {
      _repo = repo;
   }

   public override Get(int id)
   {
      if(! customerIsBeingNice) 
         throw new GoAwayException();
      return _repo.Get(id);
   }
}

I think the architect was trying to avoid the call super code smell, but since we are not requiring a call to super - in fact it really should never happen, because if you override the method, you are supposed to implement the flow you want, whereas if you call super, you can't hook into the flow that easily.

Which of these is better/worse, and why? Any alternatives?

The problem with building a good base class is that ideally, you should know exactly what kind of functionality will the derived classes need from the base class.

If you think they'll need more than is actually necessary, you'll write more code unnecessarily and it will also make the base class harder to use. On the other hand, if you think the derived classes will need less, your base class will be hard (or impossible) to use for those advanced scenarios.

For example, in your specific case, imagine you would need to call the base Get() with different IRepositorys. With your implementation, that's hard to do (though not impossible, you use a hack like IRepository wrapper where the wrapped IRepository can be switched, but ugh). With the architect's implementation, that's a trivial thing to do: just pass different IRepository to each call of InternalGet().

But again, choosing between the two depends on expectations of what the derived classes will need. If a scenario like I just described is unlikely, then your implementation is better, because it's simpler. But if that scenario happens, your implementation is clearly insufficient and the one from the architect is clearly better.

StackExchange Code Review Q#35337, answer score: 7