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

Fix maybe undefined variable offsets #23

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Mike-Hermans
Copy link

@Mike-Hermans Mike-Hermans commented Dec 20, 2022

Updates all $data[] checks to return 0 (or any other value if it was defined) for the addReadEntity method.

Should fix #18

@Mike-Hermans Mike-Hermans changed the title Fix maybe unset variables Fix maybe undefined variable offsets Dec 20, 2022
Copy link
Collaborator

@atimmer atimmer left a comment

Choose a reason for hiding this comment

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

In general a good change, but I’m seeing a lot of places where the values aren’t necessarily optional. For example, for lines I would assume the start (and maybe the end) coordinates would be mandatory.

Are you running into issues with malformed dxf files?

$thickness = $data[39] ? $data[39] : 0;
$text = new Text($data[1], $point, $data[40], $rotation, $thickness);
if ($data[72]) {
$point = [$data[10] ?? 0, $data[20] ?? 0, $data[30] ?? 0];
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don’t know about TEXT, but are these values optional for those entities?

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.

PHP 7.4 reports lots of Notices about 'Undefined offset'.. would you accept a PR fixing these?
3 participants