Prefer explicit setters over optional parameters

Spomky-Labs · Nov 5, 2022 · 0d3f92d · 0d3f92d
1 parent c705164
commit 0d3f92d
Show file tree

Hide file tree

Showing 9 changed files with 161 additions and 124 deletions.
diff --git a/src/HOTP.php b/src/HOTP.php
@@ -12,33 +12,36 @@
  */
 final class HOTP extends OTP implements HOTPInterface
 {
-    protected function __construct(null|string $secret, int $counter, string $digest, int $digits)
-    {
-        parent::__construct($secret, $digest, $digits);
-        $this->setCounter($counter);
-    }
-
     public static function create(
         null|string $secret = null,
-        int $counter = 0,
-        string $digest = 'sha1',
-        int $digits = 6
+        int $counter = self::DEFAULT_COUNTER,
+        string $digest = self::DEFAULT_DIGEST,
+        int $digits = self::DEFAULT_DIGITS
     ): self {
-        return new self($secret, $counter, $digest, $digits);
+        $htop = $secret !== null
+            ? self::createFromSecret($secret)
+            : self::generate()
+        ;
+        $htop->setCounter($counter);
+        $htop->setDigest($digest);
+        $htop->setDigits($digits);
+
+        return $htop;
     }
 
-    public static function createFromSecret(
-        string $secret,
-        int $counter = 0,
-        string $digest = 'sha1',
-        int $digits = 6
-    ): self {
-        return new self($secret, $counter, $digest, $digits);
+    public static function createFromSecret(string $secret): self
+    {
+        $htop = new self($secret);
+        $htop->setCounter(self::DEFAULT_COUNTER);
+        $htop->setDigest(self::DEFAULT_DIGEST);
+        $htop->setDigits(self::DEFAULT_DIGITS);
+
+        return $htop;
     }
 
-    public static function generate(int $counter = 0, string $digest = 'sha1', int $digits = 6): self
+    public static function generate(): self
     {
-        return new self(self::generateSecret(), $counter, $digest, $digits);
+        return self::createFromSecret(self::generateSecret());
     }
 
     public function getCounter(): int
@@ -72,7 +75,7 @@ public function verify(string $otp, null|int $counter = null, null|int $window =
         return $this->verifyOtpWithWindow($otp, $counter, $window);
     }
 
-    protected function setCounter(int $counter): void
+    public function setCounter(int $counter): void
     {
         $this->setParameter('counter', $counter);
     }

diff --git a/src/HOTPInterface.php b/src/HOTPInterface.php
@@ -6,6 +6,8 @@
 
 interface HOTPInterface extends OTPInterface
 {
+    public const DEFAULT_COUNTER = 0;
+
     /**
      * The initial counter (a positive integer).
      */
@@ -16,6 +18,9 @@ public function getCounter(): int;
      *
      * If the secret is null, a random 64 bytes secret will be generated.
      *
+     * @param null|non-empty-string $secret
+     * @param non-empty-string $digest
+     *
      * @deprecated Deprecated since v11.1, use ::createFromSecret or ::generate instead
      */
     public static function create(
@@ -25,20 +30,5 @@ public static function create(
         int $digits = 6
     ): self;
 
-    /**
-     * Create a TOTP object from an existing secret.
-     *
-     * @param non-empty-string $secret
-     */
-    public static function createFromSecret(
-        string $secret,
-        int $counter = 0,
-        string $digest = 'sha1',
-        int $digits = 6
-    ): self;
-
-    /**
-     * Create a new HOTP object. A random 64 bytes secret will be generated.
-     */
-    public static function generate(int $counter = 0, string $digest = 'sha1', int $digits = 6): self;
+    public function setCounter(int $counter): void;
 }
diff --git a/src/OTP.php b/src/OTP.php
@@ -17,11 +17,12 @@ abstract class OTP implements OTPInterface
 {
     use ParameterTrait;
 
-    protected function __construct(null|string $secret, string $digest, int $digits)
+    /**
+     * @param non-empty-string $secret
+     */
+    protected function __construct(string $secret)
     {
         $this->setSecret($secret);
-        $this->setDigest($digest);
-        $this->setDigits($digits);
     }
 
     public function getQrCodeUri(string $uri, string $placeholder): string

diff --git a/src/OTPInterface.php b/src/OTPInterface.php
@@ -6,6 +6,34 @@
 
 interface OTPInterface
 {
+    public const DEFAULT_DIGITS = 6;
+
+    public const DEFAULT_DIGEST = 'sha1';
+
+    /**
+     * Create a OTP object from an existing secret.
+     *
+     * @param non-empty-string $secret
+     */
+    public static function createFromSecret(string $secret): self;
+
+    /**
+     * Create a new OTP object. A random 64 bytes secret will be generated.
+     */
+    public static function generate(): self;
+
+    /**
+     * @param non-empty-string $secret
+     */
+    public function setSecret(string $secret): void;
+
+    public function setDigits(int $digits): void;
+
+    /**
+     * @param non-empty-string $digest
+     */
+    public function setDigest(string $digest): void;
+
     /**
      * @return string Return the OTP at the specified timestamp
      */

diff --git a/src/ParameterTrait.php b/src/ParameterTrait.php
@@ -121,6 +121,21 @@ public function setParameter(string $parameter, mixed $value): void
         }
     }
 
+    public function setSecret(string $secret): void
+    {
+        $this->setParameter('secret', $secret);
+    }
+
+    public function setDigits(int $digits): void
+    {
+        $this->setParameter('digits', $digits);
+    }
+
+    public function setDigest(string $digest): void
+    {
+        $this->setParameter('algorithm', $digest);
+    }
+
     /**
      * @return array<string, callable>
      */
@@ -161,21 +176,6 @@ protected function getParameterMap(): array
         ];
     }
 
-    private function setSecret(null|string $secret): void
-    {
-        $this->setParameter('secret', $secret);
-    }
-
-    private function setDigits(int $digits): void
-    {
-        $this->setParameter('digits', $digits);
-    }
-
-    private function setDigest(string $digest): void
-    {
-        $this->setParameter('algorithm', $digest);
-    }
-
     private function hasColon(string $value): bool
     {
         $colons = [':', '%3A', '%3a'];

diff --git a/src/TOTP.php b/src/TOTP.php
@@ -12,40 +12,39 @@
  */
 final class TOTP extends OTP implements TOTPInterface
 {
-    protected function __construct(null|string $secret, int $period, string $digest, int $digits, int $epoch = 0)
-    {
-        parent::__construct($secret, $digest, $digits);
-        $this->setPeriod($period);
-        $this->setEpoch($epoch);
-    }
-
     public static function create(
         null|string $secret = null,
-        int $period = 30,
-        string $digest = 'sha1',
-        int $digits = 6,
-        int $epoch = 0
+        int $period = self::DEFAULT_PERIOD,
+        string $digest = self::DEFAULT_DIGEST,
+        int $digits = self::DEFAULT_DIGITS,
+        int $epoch = self::DEFAULT_EPOCH
     ): self {
-        return new self($secret, $period, $digest, $digits, $epoch);
+        $totp = $secret !== null
+            ? self::createFromSecret($secret)
+            : self::generate()
+        ;
+        $totp->setPeriod($period);
+        $totp->setDigest($digest);
+        $totp->setDigits($digits);
+        $totp->setEpoch($epoch);
+
+        return $totp;
     }
 
-    public static function createFromSecret(
-        string $secret,
-        int $period = 30,
-        string $digest = 'sha1',
-        int $digits = 6,
-        int $epoch = 0
-    ): self {
-        return new self($secret, $period, $digest, $digits, $epoch);
+    public static function createFromSecret(string $secret): self
+    {
+        $totp = new self($secret);
+        $totp->setPeriod(self::DEFAULT_PERIOD);
+        $totp->setDigest(self::DEFAULT_DIGEST);
+        $totp->setDigits(self::DEFAULT_DIGITS);
+        $totp->setEpoch(self::DEFAULT_EPOCH);
+
+        return $totp;
     }
 
-    public static function generate(
-        int $period = 30,
-        string $digest = 'sha1',
-        int $digits = 6,
-        int $epoch = 0
-    ): self {
-        return new self(self::generateSecret(), $period, $digest, $digits, $epoch);
+    public static function generate(): self
+    {
+        return self::createFromSecret(self::generateSecret());
     }
 
     public function getPeriod(): int
@@ -118,11 +117,16 @@ public function getProvisioningUri(): string
         return $this->generateURI('totp', $params);
     }
 
-    protected function setPeriod(int $period): void
+    public function setPeriod(int $period): void
     {
         $this->setParameter('period', $period);
     }
 
+    public function setEpoch(int $epoch): void
+    {
+        $this->setParameter('epoch', $epoch);
+    }
+
     /**
      * @return array<string, callable>
      */
@@ -161,11 +165,6 @@ protected function filterOptions(array &$options): void
         ksort($options);
     }
 
-    private function setEpoch(int $epoch): void
-    {
-        $this->setParameter('epoch', $epoch);
-    }
-
     private function timecode(int $timestamp): int
     {
         return (int) floor(($timestamp - $this->getEpoch()) / $this->getPeriod());

diff --git a/src/TOTPInterface.php b/src/TOTPInterface.php
@@ -6,36 +6,30 @@
 
 interface TOTPInterface extends OTPInterface
 {
+    public const DEFAULT_PERIOD = 30;
+
+    public const DEFAULT_EPOCH = 0;
+
     /**
      * Create a new TOTP object.
      *
      * If the secret is null, a random 64 bytes secret will be generated.
      *
+     * @param null|non-empty-string $secret
+     * @param non-empty-string $digest
+     *
      * @deprecated Deprecated since v11.1, use ::createFromSecret or ::generate instead
      */
     public static function create(
         null|string $secret = null,
-        int $period = 30,
-        string $digest = 'sha1',
-        int $digits = 6
+        int $period = self::DEFAULT_PERIOD,
+        string $digest = self::DEFAULT_DIGEST,
+        int $digits = self::DEFAULT_DIGITS
     ): self;
 
-    /**
-     * Create a TOTP object from an existing secret.
-     *
-     * @param non-empty-string $secret
-     */
-    public static function createFromSecret(
-        string $secret,
-        int $period = 30,
-        string $digest = 'sha1',
-        int $digits = 6
-    ): self;
+    public function setPeriod(int $period): void;
 
-    /**
-     * Create a new TOTP object. A random 64 bytes secret will be generated.
-     */
-    public static function generate(int $period = 30, string $digest = 'sha1', int $digits = 6): self;
+    public function setEpoch(int $epoch): void;
 
     /**
      * Return the TOTP at the current time.