Creating multiple conditions in objects preventing nesting

Question

My class is a factory-method that allows to instantiate it only when the parameter $type (string) is "regular or premium" and when parameter $months (integer) is lower than 6.

If $months is lower than 6, an error prompts and the subscription instance is not created. I've created an if condition if (class_exists($suscription)) inside another if condition if ($months < 6) knowing it is a bad practice.

How can I improve the way I am creating the conditions (maybe creating a different method for $months validation)?

 <?php
 class Subscription {
 protected $months;
 
 public function __construct($months) {
 $this->months = $months;
 }
 
 public static function create($type, $months) {
 $suscription = ucwords($type);
 
 try {
 if ($months < 6) {
 throw new Exception("<p style='color: red;'>The minimum must be 6 months</p>");
 } else {
 try {
 if (class_exists($suscription)) {
 return new $suscription($months);
 } else {
 throw new Exception("<p style='color: red;'>Subscription type " . $type . " not available</p>");
 }
 } catch (Exception $ex) {
 echo $ex->getMessage();
 }
 }
 } catch (Exception $ex) {
 echo $ex->getMessage();
 }
 }
 // I tried this first, but didn't worked because it never stoped the instantiation of the new $suscription($months);
 private static function minimumMonths($months) {
 try {
 if ($months < 6) {
 throw new Exception("<p style='color: red;'>The minimum of months must be 6</p>");
 } 
 } catch (Exception $ex) {
 echo $ex->getMessage();
 }
 }
 }
 class RegularSubscription extends Subscription {
 public function __construct($months) {
 parent::__construct($months);
 print "<p style='color: green;'>Regular subscription has been created, it has " . $this->months . " months</p>";
 }
 }
 
 class PremiumSubscription extends Subscription {
 public function __construct($months) {
 parent::__construct($months);
 print "<p style='color: green;'>Premium subscription has been created, it has " . $this->months . " months</p>";
 }
 }
 
 $subscription = Subscription::create('Premiumsubscription', 3);

• slepic: Hello, can you explain purpose of the subscription classes and why you need multiple of them?
• Ricardo Castañeda: @slepic Hello, I'm learning how design patterns work, to be specific, the factory method.

Answer 1

You're probably overthinking this. Do you really need multiple classes? Unless those subscriptions have some distinct behaviour, there is no need for more than properties on just one class.

Don't just catch exceptions where you throw them. There is no reason to throw them in the first place if you have code that can handle the case right a few lines below. But echoing something really isn't the right logic for handling errors.

You should not print/echo anything inside those classes - I get that it's for debugging, but there is no place for this in production code. And it's better to use a proper debugging tool like xdebug.

You could use some modern features of PHP to simplify your code a lot.

You can use constructor promotion.

You can define enum for the types of subscriptions. It will allow you to separate the place where an invalid type error is thrown and where an invalid subscription setup error is thrown.

Do not implement the checks in the static factory if your constructor is public. Either make the constructor private or move all necesary checks inside the contructor.

Use appropriate typing wherever possible.

Make your classes readonly if possible.

The more explicit you are about the types and intents of your classes/methods/functions, the better for anyone using them or just reading the code.

enum SubscriptionType : string
{
 case regular = 'regular';
 case premium = 'premium';
}
final class InvalidSubscriptionDurationException extends \InvalidArgumentException
{
}
final readonly class Subscription
{
 public function __construct(
 public SubscriptionType $type,
 public int $months,
 )
 {
 if ($this->months < 6) {
 throw new InvalidSubscriptionDurationException('Must be at least 6 months');
 }
 }
}
$subscription = new Subscription(SubscriptionType::regular, 12);
$subscription = new Subscription(SubscriptionType::from($typeString), 12);

If different subscriptions shall have different behaviour, maybe then you could use inheritance, but one could argue you should use composition instead.

You should also avoid magically converting strings to classes. Just instantiate the class you want. Maybe different classes will need different constructor arguments...

final readonly class SubscriptionDuration
{
 public function __construct(public int $months)
 {
 if ($this->months < 6) {
 throw new InvalidSubscriptionDurationException('Must be at least 6 months');
 }
 }
}
interface SubscriptionBehaviour
{
 function behaviour(): void;
}
final readonly class RegularSubscription implements SubscriptionBehaviour
{
 public function __construct(private SubscriptionDuration $duration)
 {
 }
 function behaviour(): void
 {
 // whatever
 }
}
final readonly class PremiumSubscription implements SubscriptionBehaviour
{
 public function __construct(private SubscriptionDuration $duration)
 {
 }
 function behaviour(): void
 {
 // whatever
 }
}
final class SubscriptionFactory
{
 private function __construct() {}
 public static function create(SubscriptionType $type, SubscriptionDuration $duration): SubscriptionBehaviour
 {
 return match ($type) {
 SubscriptionType::regular => new RegularSubscription($duration), 
 SubscriptionType::premium => new PremiumSubscription($duration),
 };
 }
}
```