"Overengineered dice roller with a bad case of const-itis"

lang: PHP·23 lines

//your_submission

DiceRoller.php

<?php

declare(strict_types=1);

namespace RPGPlayground\Domain\ValueObjects;

final class Dice
{
    public const int MINIMUM_VALUE = 1;

    /**
     * @param int $sides
     * @throws \InvalidArgumentException
     */
    public function __construct(
        public int $sides,
    ) {
        if ($sides < self::MINIMUM_VALUE) {
            throw new \InvalidArgumentException('Invalid number of sides for a dice');
        }
    }
}

//detailed_analysis

Const Abuse

Using const for a minimum value that will never change, how bold, how daring, how utterly unnecessary. It's not like it's a configurable variable or something.

Invalid Access Modifier

Making the const public? What's next, making the constructor public too? Oh wait, you did. Get a grip on your access modifiers, mate.

Lack of Input Validation

You're only checking if the number of sides is less than the minimum value. What about if it's not an integer? Or if it's a really large number? You know, the kind of input that'll make your dice roller go 'oh no, I've been rolled'

//suggested_fix

DiceRoller_fixed.php

+<?php

+declare(strict_types=1);

+namespace RPGPlayground\Domain\ValueObjects;

+final class Dice

+ private const int MINIMUM_VALUE = 1;

+ public function __construct(

+ public int $sides,

+ ) {

+ if ($sides < self::MINIMUM_VALUE) {

+ throw new \InvalidArgumentException('Invalid number of sides for a dice');

+ }