Unit Testing – Testing Protected Methods Without Making Them Public

abstractionclean codepolymorphismunit testing

I need to make a sponsorship system with complex business requirements. Basically, after a user makes a payment, the system should get triggered. There are many different types of sponsoring, so I found abstractions to make my life simpler now and in the future.

To use polymorphism, I first created an abstract class that every sponsoring system would inherit from, and implement each abstract method needed.

Here's the class :

abstract class SponsorshipService
{
    /** @var UserKang */
    protected $user;
    /** @var UserKang */
    protected $sponsor;
    /** @var SponsorUser */
    protected $sponsorship;
    /** @var int */
    protected $amount;
    /** @var BillingMicroService */
    protected $billingMicroService;
    /** @var MailMicroService */
    protected $mailMicroService;

    public function __construct(BillingMicroService $billingMicroService, MailMicroService $mailMicroService)
    {
        $this->billingMicroService = $billingMicroService;
        $this->mailMicroService = $mailMicroService;
    }

    public function apply(UserKang $user, int $amount): bool
    {
        $this->user = $user;
        $this->amount = $amount;

        if (!$this->user->isSponsored()) {
            return false;
        }

        $this->sponsor = $this->user->getSponsor();
        $this->sponsorship = $this->user->getSponsorship();

        if (!$this->checkSponsorshipAppliance()) {
            return false;
        }

        $offeredCash = $this->getOfferedCash();
        $isCreditOffered = $this->offerCreditSponsorship($offeredCash);

        if (!$isCreditOffered) {
            return false;
        }

        $this->sendEmail();
        $this->updateSponsorship();
    }

    abstract protected function checkSponsorshipAppliance(): bool;

    abstract protected function getOfferedCash(): int;

    abstract protected function offerCreditSponsorship(): bool;

    abstract protected function sendEmail(): void;

    abstract protected function updateSponsorship(): void;
}

This is all good so far, but I'd like to unit tests each of these abstract methods for each SponsorshipService implementation I'll make. But I made these protected, because it doesn't seem like they have any reason to be public, and I think the less public methods a class has, the easier it is to use, and the safer it is too.

What's wrong in this design ? Clearly I don't want to unit test apply in every possible way for every implementation as it would be a nightmare as much now as it'd be in the future.

Best Answer

What's wrong in this design?

It is a subjective question, though here's some food for thought:

The constructor initializes two instance members (billingMicroService, mailMicroService), yet not all instance members — the others get initialized in apply.

When we have instance fields that can be grouped into two (or more) distinct sets of lifetimes within one class, it suggests that we have conflated what could be independent domain concepts/entities — and this suggests refactoring into two (or more) classes.  (Sometimes one set of instance fields could become parameters instead of another class.)

apply also conditionally initializes some fields suggesting possible further conflation.

Do you expect a single concrete subclass to implement all the features you are offering in the abstract base, like sending emails and offerings of cash as well as offerings of credit?

This feels like a violation of single responsibility, for one.

But for another, if you want to mix and match abilities to send emails, with different ways to offer cash, with yet differing ways of offering credit, you'll have to create a new subclass for each combination — this results in class explosion, and can be addressed by composition.

(While it is true that you can introduce abstract subclasses that offer partial functionality so you have more DRY (reuse) than re-implementing things in each concrete subclass, managing a class hierarchy to pick features that way is hard an unnecessary since this can be done more simply with composition instead.)

If you use composition instead of inheritance, I believe you'll find testing individual capabilities easier.

Related Topic