Change SetCookie constructor from private to protected

[levi-jcbs]

Good morning,

I'm extending the setCookie class in my projects to do configuration directly in the constructor. But I need to be able to call parent::__construct() to use this pattern. parent::create() doesn't work, because the create function would then call the child constructor resulting in an infinite loop.

The pull request I'm proposing is to change the __construct() method from private to protected.

Are there any objections?

Thanks for your time,
Levi Jacobs

Edit: Example for my use case:

class CartCookie extends SetCookie implements CartCookieInterface
{
	public function __construct(CartModelInterface $cart)
	{
		parent::__construct(
			"cart",
			$cart->token
		);

		$this->withHttpOnly(true)
			->withSecure(true)
			->withPath("/")
			->withExpires($cart->time + 60 * 30) // 30 minutes cookie expiry
			->withSameSite(sameSite: SameSite::lax());
	}
}

[gsteel]

Thanks for the patch @levi-jcbs, however, I'm not in favour of the proposed change and would rather see the class made final, preventing inheritance entirely.

Making the constructor protected exposes the class to BC guarantees whereas at the moment, the constructor can be freely changed.

The correct way to instantiate this class is via public, static named constructors.

Can I suggest a factory method/class instead that's specific to your project such as:

final class CartCookieFactory {
    public static function fromCartModel(CartModel $model): SetCookie
    {
        return SetCookie::create('cart', $model->token())
            ->withHttpOnly(true); // ... etc
    }
}

… And if you need specific methods on the cookie instance, decorate SetCookie along the lines of:

final class CartCookie implements Whatever {
    public function __construct(private readonly SetCookie $cookie) {}
}

Feel free to ask other maintainers for a second opinion tho 👍

[levi-jcbs]

Thanks for your answer, @gsteel.

I totally understand, that you want to keep the constructor private, I didn't think of that from your perspective.

I can't use the Factory approach you suggested, because I need specifically typed Objects (e.g. CartCookie, AuthCookie and not just SetCookie).

But the second approach is very smart, I didn't think of that! Instead of extending I'm just setting a property now:

class CartCookie implements CartCookieInterface
{
	public readonly SetCookie $figSetCookie;

	public function __construct(
		CartModelInterface $cart,
		SetCookieFactoryInterface $figSetCookieFactory  // injected via DI Container
	) {
		$this->figSetCookie = $figSetCookieFactory->create(
			"cart",
			$cart->token
		)
			->withHttpOnly(true)
			->withSecure(true)
			->withPath("/")
			->withExpires($cart->time + 60 * 30)  // 30 minutes cookie expiry
			->withSameSite(sameSite: SameSite::lax());
	}
}

Thank's for your help.