Bacon / BaconQrCode

QR Code Generator for PHP
BSD 2-Clause "Simplified" License
1.83k stars 208 forks source link

Included 'return', for ease of testing on QRCode generation. #33

Closed Diego-Brocanelli closed 7 years ago

Diego-Brocanelli commented 7 years ago

With the addition of the 'return' statement, it facilitates the creation of unit tests for QR Code generator. Allowing you to test whether the file was generated or not.

DASPRiD commented 7 years ago

Why would you need this for unit testing? First of: If code is there just for unit testing, it shouldn't be there at all. Secondly, for unit testing you can just file_get_contents() the resulting path to test it.

Diego-Brocanelli commented 7 years ago

With the inclusion of the return, there is no need to validate the file, because the function returns false if it can not create the file. Thus, it facilitates and reduces the complexity of generating tests. Anyone who consumes the lib will create their tests in the applications and with this will find it difficult to test the return that the method provides. Because it always returns null.

Diego-Brocanelli commented 7 years ago

Hence the inclusion of the return, thinking of facilitating that will consume the lib. Keeping in mind they will test your implementations on your applications.

DASPRiD commented 7 years ago

If anything, the method should throw an exception if the file cannot be created, since that shouldn't be the normal flow. Also, the method does not return null (explicitly), but void. And again, as said, there shouldn't be any unit-test-only logic in the code base.

Diego-Brocanelli commented 7 years ago

OK