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

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

Open
neekfenwick opened this issue Apr 20, 2021 · 1 comment · May be fixed by #23
Open

Comments

@neekfenwick
Copy link

Hi there, I'm running DXFighter for some very simple requirements and notice that it produces a lot of Notice level logging such as on this line:

    $thickness = $data[39] ? $data[39] : 0;

Here the '39' is specified in the spec and is not really intended to be used as a numerical index into the array, and if it doesn't exist in the array it results in a Notice. Older PHP perhaps didn't complain about this but, at least, PHP 7 does. It seems the correct code would be:

    $thickness = isset($data[39]) ? $data[39] : 0;

When running from the command line I'm finding all the Notice output annoying and would rather fix it than adjust my log levels. Would you accept a PR that tries to replace all these occurences?

I'm also not sure if the '39' should be quoted, as we're not so much referring to 39 as a number, but as a 'code' as defined in the spec. See https://images.autodesk.com/adsk/files/autocad_2012_pdf_dxf-reference_enu.pdf under "Group Code Value Types". It seems that leaving them as integers in the PHP code, and thus a sparsely allocated array, works fine, so I guess quoting the code such as $data['39'] isn't necessary.

@enjoping
Copy link
Owner

Hey, thanks for your input! You are right, the code was written with PHP version 5.6 being the newest one if I call correctly. At this time using an array index which has not been defined was not throwing a notice. But we can definitively fix this.
The isset approach is correct and you are right. The number used is indeed more of an object key than an array index. So yes, changing it to a string representative would be good.

Please feel free to provide an PR and I will merge it as soon as possible.

@Mike-Hermans Mike-Hermans linked a pull request Dec 20, 2022 that will close this issue
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 a pull request may close this issue.

2 participants