search

enjoping / DXFighter

A new DXF writer and reader in PHP which outputs AC1012 (R15) DXF files
BSD 3-Clause "New" or "Revised" License
42 stars 22 forks source link

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

Open neekfenwick opened 3 years ago

neekfenwick commented 3 years ago

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 commented 2 years ago

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.