Spelt / ZXing.Delphi

ZXing Barcode Scanning object Pascal Library for Delphi VCL and Delphi Firemonkey
Apache License 2.0
479 stars 204 forks source link

Class variables not being initialised #55

Closed SteveCorbett closed 7 years ago

SteveCorbett commented 7 years ago

There's a few issues with class variables not being initialised. Here's the fixes. patch.zip

Spelt commented 7 years ago

Could you eleborate a bit more? A far as I can tell this patch only replaces create/destroy with initialize/finalize.

With create code is only run when the object is used/created. With initialize it will run once.

SteveCorbett commented 7 years ago

On 12 Jul 2017, at 12:34 am, E Spelt notifications@github.com wrote:

Could you eleborate a bit more? A far as I can tell this patch only replaces create/destroy with initialize/finalize.

With create code is only run when the object is used/created. With initialize it will run once.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314464076, or mute the thread https://github.com/notifications/unsubscribe-auth/AFpBNXUh1ntdnBVP3RqCJId51z24Kx5nks5sM4gKgaJpZM4ON0lx.

The problem is that you are referring to class variables in class functions that are not initialised until an instance of the object is created. It is not necessary to create an object of a class in order to call a class function.

For example, it is valid to call TCode128Reader.DecodeCode without first performing a TCode128Reader.Create.

I notice this when looking at some code that crashed after using the following line of code:

TScanManager.Create(TBarcodeFormat.Auto, nil);

Using the given source code changes fixed this problem. Sorry, I haven’t traced through to find out where it may have been calling the class functions without first creating an instance of the object. I suspect that the problem is that there are descendant classes not calling the inherited constructor.

Spelt commented 7 years ago

Hi Corbin,

TCode128Reader.DecodeCode is a private function and cannot be called normally.

Do you have a failing test which you can provide? Like one in the test project? And also which version of Delphi do you use? and do you use VCL or FMX?

2017-07-12 5:52 GMT+02:00 SteveCorbett notifications@github.com:

On 12 Jul 2017, at 12:34 am, E Spelt notifications@github.com wrote:

Could you eleborate a bit more? A far as I can tell this patch only replaces create/destroy with initialize/finalize.

With create code is only run when the object is used/created. With initialize it will run once.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314464076>, or mute the thread https://github.com/notifications/unsubscribe-auth/ AFpBNXUh1ntdnBVP3RqCJId51z24Kx5nks5sM4gKgaJpZM4ON0lx.

The problem is that you are referring to class variables in class functions that are not initialised until an instance of the object is created. It is not necessary to create an object of a class in order to call a class function.

For example, it is valid to call TCode128Reader.DecodeCode without first performing a TCode128Reader.Create.

I notice this when looking at some code that crashed after using the following line of code:

TScanManager.Create(TBarcodeFormat.Auto, nil);

Using the given source code changes fixed this problem. Sorry, I haven’t traced through to find out where it may have been calling the class functions without first creating an instance of the object. I suspect that the problem is that there are descendant classes not calling the inherited constructor.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314637281, or mute the thread https://github.com/notifications/unsubscribe-auth/AMEt1Q7I7RDto3XEtRsE4ffYGNt-liPlks5sNEMOgaJpZM4ON0lx .

SteveCorbett commented 7 years ago

Hi there,

Regardless of whether a method is private or public, it should not be referencing objects that haven’t been initialised and will fail. A class function or class method can be invoked without an object of the class being instantiated. Unit ZXing.OneD.EAN13Reader might be a better example. It has a public method TEAN13Reader.DecodeMiddle that references any array whose length was being set only if a TEAN13Reader is created.

I’m using Berlin with the latest updates. I found this problem when trying to run the ScannerMApp in debug on an Android device. This caused segmentation (memory) errors to be picked up by the debugger. Funnily, the same code ran sometimes without displaying an error when not using the IDE debug, but other times it would just crash the application.

These were the only changes I had to make to make the project run without crashing.

Apart from that, I’m impressed with this project.

Regards

Steve

On 12 Jul 2017, at 4:08 pm, E Spelt notifications@github.com wrote:

Hi Corbin,

TCode128Reader.DecodeCode is a private function and cannot be called normally.

Do you have a failing test which you can provide? Like one in the test project? And also which version of Delphi do you use? and do you use VCL or FMX?

2017-07-12 5:52 GMT+02:00 SteveCorbett notifications@github.com:

On 12 Jul 2017, at 12:34 am, E Spelt notifications@github.com wrote:

Could you eleborate a bit more? A far as I can tell this patch only replaces create/destroy with initialize/finalize.

With create code is only run when the object is used/created. With initialize it will run once.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314464076>, or mute the thread https://github.com/notifications/unsubscribe-auth/ AFpBNXUh1ntdnBVP3RqCJId51z24Kx5nks5sM4gKgaJpZM4ON0lx.

The problem is that you are referring to class variables in class functions that are not initialised until an instance of the object is created. It is not necessary to create an object of a class in order to call a class function.

For example, it is valid to call TCode128Reader.DecodeCode without first performing a TCode128Reader.Create.

I notice this when looking at some code that crashed after using the following line of code:

TScanManager.Create(TBarcodeFormat.Auto, nil);

Using the given source code changes fixed this problem. Sorry, I haven’t traced through to find out where it may have been calling the class functions without first creating an instance of the object. I suspect that the problem is that there are descendant classes not calling the inherited constructor.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314637281, or mute the thread https://github.com/notifications/unsubscribe-auth/AMEt1Q7I7RDto3XEtRsE4ffYGNt-liPlks5sNEMOgaJpZM4ON0lx .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314663992, or mute the thread https://github.com/notifications/unsubscribe-auth/AFpBNQpl4-Acq4QqtVwYOTMq4nYUziuqks5sNGLEgaJpZM4ON0lx.

Spelt commented 7 years ago

Thanks,

I will include the patch. It will also improve speed a bit.

2017-07-12 8:39 GMT+02:00 SteveCorbett notifications@github.com:

Hi there,

Regardless of whether a method is private or public, it should not be referencing objects that haven’t been initialised and will fail. A class function or class method can be invoked without an object of the class being instantiated. Unit ZXing.OneD.EAN13Reader might be a better example. It has a public method TEAN13Reader.DecodeMiddle that references any array whose length was being set only if a TEAN13Reader is created.

I’m using Berlin with the latest updates. I found this problem when trying to run the ScannerMApp in debug on an Android device. This caused segmentation (memory) errors to be picked up by the debugger. Funnily, the same code ran sometimes without displaying an error when not using the IDE debug, but other times it would just crash the application.

These were the only changes I had to make to make the project run without crashing.

Apart from that, I’m impressed with this project.

Regards

Steve

On 12 Jul 2017, at 4:08 pm, E Spelt notifications@github.com wrote:

Hi Corbin,

TCode128Reader.DecodeCode is a private function and cannot be called normally.

Do you have a failing test which you can provide? Like one in the test project? And also which version of Delphi do you use? and do you use VCL or FMX?

2017-07-12 5:52 GMT+02:00 SteveCorbett notifications@github.com:

On 12 Jul 2017, at 12:34 am, E Spelt notifications@github.com wrote:

Could you eleborate a bit more? A far as I can tell this patch only replaces create/destroy with initialize/finalize.

With create code is only run when the object is used/created. With initialize it will run once.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314464076 , or mute the thread https://github.com/notifications/unsubscribe-auth/ AFpBNXUh1ntdnBVP3RqCJId51z24Kx5nks5sM4gKgaJpZM4ON0lx.

The problem is that you are referring to class variables in class functions that are not initialised until an instance of the object is created. It is not necessary to create an object of a class in order to call a class function.

For example, it is valid to call TCode128Reader.DecodeCode without first performing a TCode128Reader.Create.

I notice this when looking at some code that crashed after using the following line of code:

TScanManager.Create(TBarcodeFormat.Auto, nil);

Using the given source code changes fixed this problem. Sorry, I haven’t traced through to find out where it may have been calling the class functions without first creating an instance of the object. I suspect that the problem is that there are descendant classes not calling the inherited constructor.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/55# issuecomment-314637281, or mute the thread https://github.com/notifications/unsubscribe-auth/ AMEt1Q7I7RDto3XEtRsE4ffYGNt-liPlks5sNEMOgaJpZM4ON0lx .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314663992>, or mute the thread https://github.com/notifications/unsubscribe- auth/AFpBNQpl4-Acq4QqtVwYOTMq4nYUziuqks5sNGLEgaJpZM4ON0lx.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-314669084, or mute the thread https://github.com/notifications/unsubscribe-auth/AMEt1TE_vkDi49qiJcNtxXEQYqwNlqM0ks5sNGoqgaJpZM4ON0lx .

Spelt commented 7 years ago

Its included. Thanks for contributing!

SteveCorbett commented 7 years ago

You’re welcome! Thanks again for sharing the project.

On 14 Jul 2017, at 4:01 pm, E Spelt notifications@github.com wrote:

Its included. Thanks for contributing!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Spelt/ZXing.Delphi/issues/55#issuecomment-315277846, or mute the thread https://github.com/notifications/unsubscribe-auth/AFpBNZs_JOMRdyZOhTNrw6rnKvkRP8Ptks5sNwQ8gaJpZM4ON0lx.

SteveCorbett commented 7 years ago

Dang, I found another one. I've also made some minor changes for compiling under iOS. patch1.zip

SteveCorbett commented 7 years ago

And one more... patch2.zip