Closed emanuele45 closed 2 years ago
Uh, what can I say... I have no idea what Twig is :tongue:
Looking (at a quick glimpse) at the diff, I much like the own folder for the template and the clear separation of the template in more "granular" parts (instead of a single file as before) Also see that you are replacing superglobals here and there ;) Although a few are still alive --> $import->setupScripts($_REQUEST);
Overall, it's a bit like you said: another dependency. Although that should not be a major concern, given how widespread it seems to be ;-)
Uh, what can I say... I have no idea what Twig is :tongue:
http://lmgtfy.com/?q=twig+php :stuck_out_tongue_winking_eye:
Actually, I forgot another cons: the fact that the number of files adds up. To avoid that, tough, I really would like to move to a phar archive: https://github.com/emanuele45/openimporter/compare/moar...emanuele45:phar2 That last commit on the branch is just for the show and probably not worth, but at least using phar everything is packed into maximum 2 files: easier to upload and handle (also when removing OI).
Also see that you are replacing superglobals here and there ;) Although a few are still alive --> $import->setupScripts($_REQUEST);
As I said in the other issue you cannot remove them entirely (well, actually with the filter_input stuff in php 5.2+ you could), the point is to try to use them in only one place (i.e. at the beginning) and pass them as arguments or wrap them in classes, so that you do not depend directly on the superglobal, but rather on a local variable passed as argument of a function or property of a class, etc. ;)
I had a moment of insanity (I'm not implying I am mentally sane in normal condition LOL) and played with Twig: https://github.com/emanuele45/openimporter/compare/development...twig just to have some fun, so the question is: what would you think about using Twig for the template?
pros
https://scrutinizer-ci.com/g/emanuele45/openimporter/code-structure/twig/class/OpenImporter%5CCore%5CTemplate
https://scrutinizer-ci.com/g/emanuele45/openimporter/code-structure/moar/class/OpenImporter%5CCore%5CTemplate
cons
To me it's the same, and I already prepared another branch with the non-twig improvements present in the twig branch: https://github.com/emanuele45/openimporter/compare/development...moar