Skip to content

✨ DateTimeCollection #140

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Edhrendal
Copy link
Contributor

@Edhrendal Edhrendal commented May 8, 2024

Tested! upon \DateTime

✨ Features

  • Define containsDateTimeValue() function for all DateTimeInterfaceCollection
    • Verify upon "date time + timezone" rather than object hash
    • Same for nullable \DateTimeInterface

@Edhrendal Edhrendal force-pushed the feat-datetime_collection branch 2 times, most recently from f4af396 to c3ce9f0 Compare May 25, 2024 18:21
@Edhrendal Edhrendal requested a review from steevanb July 25, 2024 11:19
@Edhrendal Edhrendal force-pushed the feat-datetime_collection branch from fb4f00b to 293f0d9 Compare September 9, 2024 14:22
@Edhrendal
Copy link
Contributor Author

Refonte de la PR en prenant en compte les classes générées de la 6.2.0 .

✅ Tests unitaires effectués seulement sur les \DateTime (pas sur les \DateTimeInterfaceCollection ou les DateTimeImmutable). À faire également ?

@Edhrendal Edhrendal force-pushed the feat-datetime_collection branch 2 times, most recently from f9b3387 to cb62abb Compare September 9, 2024 14:26

trait ContainsDateTimeNullableValueTrait
{
public function containsDateTimeValue(\DateTime $dateTime): bool
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi on n'utilise pas \DateTImeInterface dans ce trait, mais on l'utilise dans l'autre ?


namespace Steevanb\PhpCollection\ObjectCollection\DateTime;

trait ContainsDateTimeStrictValueTrait
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

j'ai pas précisé ailleurs dans la lib quand on n'a que le typage demandé, c'est quand on autorise null que j'ai précisé

Suggested change
trait ContainsDateTimeStrictValueTrait
trait ContainsDateTimeValueTrait

{
foreach ($this->toArray() as $value) {
if (
$value === null
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

je me demande à quel point on peut ne faire qu'un seul trait, qui gère tous les cas.

ici on pourrait faire un $value instanceof \DateTime, et ne plus se soucier du nullable ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est pour qu'une collection "stricte" n'ait pas besoin de faire une vérification complémentaire.

* ✅ **Tested!** upon `\DateTime`
* ✅ **Tested!** upon `\DateTimeImmutable`

✨ Features
==========

* Define `containsDateTimeValue()` function for all `DateTimeInterfaceCollection`
  - Verify upon "date time + timezone" rather than object hash
  - Same for nullable `\DateTimeInterface`
@Edhrendal Edhrendal force-pushed the feat-datetime_collection branch from cb62abb to 6711335 Compare September 9, 2024 16:45
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

Successfully merging this pull request may close these issues.

2 participants