I don't think you're taking advantage of the library you're using. The Carbon
class extends the standardDateTime
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.