Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why is Duration no longer final ? #2

Closed
nyamsprod opened this issue Jan 27, 2019 · 3 comments
Closed

Why is Duration no longer final ? #2

nyamsprod opened this issue Jan 27, 2019 · 3 comments

Comments

@nyamsprod
Copy link

@jeromegamez nice package why is the Duration class no longer final ? Seems to me that it is a Value Object and therefore should be final 🤔

@jeromegamez
Copy link
Owner

That's a good question that I've asked myself as well.

In 1.x the class was final, but extensible through the spatie/macroable package, in case somebody would want to extend the class, e.g. with inDays(), inMinutes() or similar helper functions that I perhaps would not be willing to implement in the basse class. After releasing it, it felt wrong to me - having a final class that's still open for extension through "cheating" :).

So I decided to see the Duration class within the package more as an open utility class than a strict value object. In a project that uses the class, a developer can for example to something like this

<?php

namespace App;

use Gamez\Duration;

final class Duration extends \Gamez\Duration
{
    public function inMinutes(): int
    {
        $now = new DateTimeImmutable();
        $then = $now->add($this->toDateInterval());

        $durationInSeconds = $now->diff($then, true);

        return intdiv($durationInSeconds, 60);
    }
}

I am planning to add more features to the class (perhaps even those helper methods), and as soon as I think that the class is feature complete, I will make it final again and create a new major release.

I hope this makes sense and answers your question.

@nyamsprod
Copy link
Author

Making your class final will help you down the road when adding new functionalities that requires messing up your internal code. Currently any changes to your internal code is potentially a BC break because the class is not final. This was discuss in length in a specific issue of League\Period before v4 release

Also another proof is because I made Period final I was able to add full boundary type support without any BC break 😄 because the internal was heavily rewrote. Otherwise I would have add multiple method marked as deprecated left because I would have potentially broke the code of someone using them via extension.

TL;DR: You should favor composition over inheritance and make your class final.

My 2 cents

@jeromegamez
Copy link
Owner

jeromegamez commented Jan 28, 2019

You're right. Well then, that will be a fast iteration to a 3.x release :D. Thank you for your input!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants