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

Generate setters for Datetime and fixed point decimal that check if the value has *really* changed #1052

Open
allan-simon opened this issue Jan 20, 2022 · 0 comments

Comments

@allan-simon
Copy link

Summary

maker bundle should generate sensible setters to avoid people falling in a common pitfall

Context

As of January 2022, doctrine has a 'logical but confusing' behaviour

doctrine/orm#5542
doctrine/orm#1924

it's because of this line https://github.com/doctrine/orm/blob/bd949312016be418c36603d89951b9641f79ffdb/lib/Doctrine/ORM/UnitOfWork.php#L660

that does a === comparison, so for an object, it compares the reference and not the value
so admitting you have an entity Employee with arrival date 2021-01-01 and firstname robert

doing

$employee->setFirstname('robert');
$em->persist($employee);
$em->flush()

-> it does not generate a SQL request because the value was robert and it hasn't changed

but going

$employee->setArrivalDate(new \Datetime('2021-01-01'));
$em->persist($employee);
$em->flush()

-> it generates a new request because this is a new object though the date hasn't changed

a workaround waiting an official solution from doctrine/orm#5542

is to do a check manually in the setter (so we do it once and for all) and not change if "equal" in a logical (same datetime/same amount)

currently at least decimal and datetime fields are concerned (datetime because a new reference is created, decimal because a strict string comparison is not sufficient , i.e '0' and '0.0' will generate a UPDATE though they're actually a real value )

Proposed solutions

  1. simply add a warning comment in the setter
    public function setLinkedAt(\DateTimeInterface $linkedAt): self
    {
             // warning, this may generate unecessary update: see *link*
            $this->linkedAt = $linkedAt;

            return $this;
    }
2. we could more opinionated (like we are for some other types that generates more complex setters already) 

we could generate

    public function setLinkedAt(\DateTimeInterface $linkedAt): self
    {
        // we do this because :  *link explaining the issue* 
        if ($this->linkedAt != $linkedAt) {
            $this->linkedAt = $linkedAt;
        }
     
        return $this;
    }
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

1 participant