2
\$\begingroup\$

I have an abstract Badge class, every class extending this class should always correctly set the incoming name (string) and achievedAt (date) fields.

There might be other classes that extend the Badge class, which will get passed different types of values in their constructor (see examples below), the most important thing is that each of these classes extending the badge class should have the correct type of value for name and achievedAt

I currently have the following structure to achieve this:

<?php

namespace App\Domain\Badge;

use Carbon\Carbon;

abstract class Badge
{
    private string $name;
    private Carbon $achievedAt;

    public function __construct(string $name, Carbon $achievedAt)
    {
        $this->name = $name;
        $this->achievedAt = $achievedAt;
    }

    public function getName(): string
    {
        return $this->name;
    }

    public function getAchievedAt(): Carbon
    {
        return $this->achievedAt;
    }
}

This is the KilometerBadge, extending the abstract Badge

<?php

namespace App\Domain\Badge;

use Carbon\Carbon;

class KilometerBadge extends Badge
{
    public function __construct(int $kilometers, $activity)
    {
        $name = 'kilometer-badge-' . $kilometers;
        $achievedAt = Carbon::parse($activity['start_date']);

        parent::__construct($name, $achievedAt);
    }
}

This is the ActivityNumberBadge, which builds the name using a different parameter

<?php

namespace App\Domain\Badge;

use Carbon\Carbon;

class ActivityNumber extends Badge
{
    public function __construct(array $activities, $activity)
    {
        $name = 'kilometer-badge-' . count($activities);
        $achievedAt = Carbon::parse($activity['start_date']);

        parent::__construct($name, $achievedAt);
    }
}

I'm wondering if this is the best way to go about this? Thank you!

\$\endgroup\$
2
\$\begingroup\$

I don't think you're taking advantage of the library you're using. The Carbon class extends the standard DateTime class, and as per the documentation, it can handle the instantiation of a Carbon object from a string representation, an integer timestamp, or any object implementing the DateTimeInterface interface. In particular, the Carbon::parse method is really just a wrapper for the constructor, which parses its argument anyways.

Taking advantage of this built-in capability, you could define the Badge class constructor as follows.

public function __construct(string $name, $achievedAt)
{
    $this->name = $name;
    $this->achievedAt = new Carbon($achievedAt);
}

You could also set the default value of the $achievedAt parameter to be 'now', which Carbon would correctly interpret and represent as the current date and time.

public function __construct(string $name, $achievedAt = 'now')
{
    $this->name = $name;
    $this->achievedAt = new Carbon($achievedAt);
}

This has the added benefit of removing the onus of adding the date parsing boilerplate from the caller, and they can now simply pass in the date argument if and when they need to (I'm assuming it's possible for user events to trigger achievements that merit badges, thus circumventing the need to parse a date-time string in those instances).

<?php

namespace App\Domain\Badge;

use Carbon\Carbon;

class KilometerBadge extends Badge
{
    /**
     * Kilometer Badge Constructor
     *
     * @param int $kilometers Kilometers walked or something.
     * @param array $activity Associative-array containing activity info.
     */
    public function __construct(int $kilometers, array $activity)
    {
        parent::__construct('kilometer-badge-' . $kilometers, $activity['start_date']);
    }
}

The creation of the ActivityNumber class is likewise simplified.

<?php

namespace App\Domain\Badge;

use Carbon\Carbon;

class ActivityNumber extends Badge
{
    public function __construct(array $activities, $activity)
    {
        parent::__construct('kilometer-badge-' . count($activities), $activity['start_date']);
    }
}

Note that if the Carbon::parse method fails, it will throw an InvalidArgumentException, which you need to make sure you're checking for.

With that being said, if the "kilometer-badge-x" badge you're creating in both classes is the same idea being created with two different classes, you might want to refactor.

If you had multiple badge types (BlueBadge and GreenBadge, for example), then each would have its own class.

class BlueBadge extends Badge
{
    //
}

class GreenBadge extends Badge
{
    //
}

If you wanted multiple ways of constructing a single badge type, you could look into the Factory pattern. In this case, you could have a class BlueBadgeFactory in which you define two static methods, createFromKilometers and createFromActivities, both of which would return an instance of a BlueBadge object.

class BlueBadgeFactory
{
    public static function createFromKilometers(int $kilometers, string $achievedAt) : BlueBadge
    {
        //
    }

    public static function createFromActivities(array $activities, array $activity) : BlueBadge
    {
        //
    }
}

I'm not familiar with the project you're working on, but it might be helpful to create a base AbstractBadgeFactory class from which each concrete badge factory class inherits some functionality.

Now that you've represented each badge with a single, distinct class with each badge type inheriting from a single Badge superclass, you can now more simply take advantage of polymorphism (you don't have to check whether an object is an instance of the KilometerBadge or the ActivityNumber badge, for instance - pun intended).

Here's a pretty contrived example to write a function that renders an array of badges.

function renderBadges(array $badges) : void
{
    foreach ($badges as $badge)
    {
        renderBadge($badge);
    }
}

The above function simply calls the following renderBadge function, which takes a Badge argument and renders it.

function renderBadge(Badge $badge) : void
{
    $badge->render();
}

This assumes, of course, that you actually define a method render in the Badge class. By defining it as abstract, you would force each badge type to override it, thereby providing a unique implementation per badge type.

The reason this example is contrived is because you don't want a render function on your Badge objects, since it presupposes the output format. Instead, you might define a BadgeWriter superclass, some subclasses like BadgeJSONWriter and BadgeHTMLWriter for API and Browser requests, respectively, and then you would pass in the badge in question to the writer object so that each writer badge object simply contains badge information and each writer object just takes that information and simply writes it to a specific format.

I hope this helps; comments, corrections, questions, and feedback are always welcome.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for this post! That's solid advice on Carbon, I forgot it can parse about anything so it's a good idea to have the abstract class handle it. \$\endgroup\$ – Miguel Stevens Sep 24 '20 at 5:04
  • \$\begingroup\$ The Factory pattern is perfect for this, completely forgot, that way I can remove the constructor from the individual classes extending the Badge class, the factory now creates the name from the given data, I hope that's correct.. \$\endgroup\$ – Miguel Stevens Sep 24 '20 at 5:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.