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

feat: Remove redundant eloquent queries with no geometry changes #65

Merged

Conversation

adamczykpiotr
Copy link
Contributor

$generator = app()->make(\Clickbar\Magellan\IO\Generator\WKB\WKBGenerator::class);
$parser = app()->make(\Clickbar\Magellan\IO\Parser\WKB\WKBParser::class);

// Parse & generate original geometry
// Geometry internally remains unchanged but object reference has changed
$model->geometry = $parser->parse(
    $generator->generate($model->geometry)
);

Before:

> $model->isDirty()
= true

After:

> $model->isDirty()
= false

@saibotk
Copy link
Member

saibotk commented Dec 9, 2023

Oh jeah we noticed this too and actually resolved this whole problem by rewriting the casting to casters in the upcoming version. Which will probably gain traction this holiday season again.
But ill talk with @ahawlitschek if we would include this as a fix for the 1.x release.

So thank you very much 👍

Copy link
Member

@saibotk saibotk left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Note that empty geometries will always be non equal due to the NAN usage.
Because NAN != NAN in PHP.

But I think this is good as is.

Thank you very much and sorry for the delay!

Copy link
Member

@ahawlitschek ahawlitschek left a comment

Choose a reason for hiding this comment

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

Looks good to me too.
Sorry for the delay!

@ahawlitschek ahawlitschek merged commit 02b9eef into clickbar:main Jan 19, 2024
18 checks passed
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.

4 participants