xrTor
commented
6 years ago can you specify all changes and plans?
Open TJSoler opened 6 years ago
xrTor
commented
6 years ago can you specify all changes and plans?
itismadness
commented
6 years ago Most of the "plans" as they are are written on our internal git instance or are in fixing known bugs or implementing approved suggestions from the forums, but minimally it's following the Strangler Pattern to "rewrite" Gazelle into something that's far more pleasant to read and write, but doing it only as things get properly fixed. Here's some of the "bigger" projects I've written about internally to staff and have thought about, but it's in no way exhaustive:
Take for example moving stuff out of classes/ into app/ (and following PSR-4), it's more that I want to properly separate out the truly static classes/functions (that don't rely on globals) and then rewrite the stuff that uses globals to be a properly instantiated class that has its dependencies injected instead of referencing the global scope. The router I'm writing, in the next push, I'm adding authorization code to it (to take the place of authorize()), and I'm going to do it by making G::$LoggedUser['AuthKey'] something that gets passed into the router instantiation, and then use that class variable. That way things can be tested within the app/ folder.
I'd like to move the view logic out of sections/ code and into proper Twig templates that are used. Mixing presentation and business logic is an awful anti-pattern.
Replace certain files in classes/ with existing composer packages. Examples of this would the code to generate QR codes or get the user's browser and OS. Let's move to something that someone else has written already and we can rely on to keep itself up-to-date. Additionally, break certain components out into composer packages as makes sense, for example the Logchecker, to allow other sites easier integration into their codebase.
Make the database a) MySQL strict compliant, b) make the initial DB migration fully utilize the internal classes and not straight MySQL queries, c) implement and use foreign keys across all tables, d) add primary keys to tables that don't have them.
Rewrite the sections/ that have a ton of logic in their index.php. A real bad offender of this is sections/login/index.php.
I'll look into seeing how hard it would be to make a bot to post stuff from our internal repo here (and vice versa) since that'd only help the lay person to contribute.
Empornium
commented
6 years ago Sounds a lot like what we've been doing with Luminance, will be interesting to compare notes.
TJSoler
commented
6 years ago That is pretty interesting.
I'm only pasting this here now, because I spent some time during my coffee break writing it this morning and now I feel like I don't want it to go to waste :P . But I see now the direction you are going. Also, I think this (that is from the mlaphp book) has some interesting points that could be applied anyway.
I understand the way of thinking of refactoring by implementing the new stuff and getting the old one "forgotten", but I believe it could also be interesting to have at least a better OOP base to work with.
If I had to do this myself, as I said, I have some good experiences in the past following the "Modernizing Legacy Applications in PHP" book. To kind of summarize the steps (bear in mind it is going to be long... it is a summary of the book steps):
I would start by deciding the new coding standards (my vote is for PSR2) and setting up a CONTRIBUTING.MD file. This file should explain the goals, and help newcomers get started with the contributing process.
This entails moving all the classes into the app folder and renaming them according to the desired namespace. Applicant class -> app\Applicant.php -> Gazelle\Applicant
This should be done one by one, checking afterward that everything still works as expected. It can entail a bit more than just moving. Making sure the classes inside can be reached (correct namespaces), and refactoring every use of that class so it uses the namespaced version of it, and not the old one. I'm actually doing that on the develop branch of my fork.
Refactor all globals to use Dependency Injection. This entails moving all the global vars to be parameters into class constructors, also all the superglobals ($_POST, $_SESSION, $_SERVER) should be refactored into a class that gets shared by the application. Like a Request class.
Extract "one-time creation" to dependency injection. This entails checking the classes for any new keyword, if what we are instantiating is a new Exception or something from the SPL, we may want to just leave it like that, but if it is something that should be moved into a Factory, do it.
I wouldn't write Unit tests at this point, but I would spend the most time possible writing characterization and / functional tests. This tests would test the functionality of the website as a whole. Maybe check out and evaluate Codeception.
I would take this time to also set up some automated things on the github, like travis / style-ci. After this step, theoretically, should be easier to see if you have broken anything while refactoring...
This step creates classes (Gateways) that deal only with the getting information out of the database. I tend to think about them, even if I'm mostly wrong, as Database Models. These are the main "forces" writing and reading from the database.
This should be where the real work begins... This entails moving all of the logic that is on those generic classes that have moved around in the past steps, and that is mostly associated and around the Gateway classes, we created on the last step, into its own class.
At the same time this is going on, the logic should be simplified and maybe if applies, modernized to a php7 code base. At this point, I WOULDN'T change any of the logic. Even if we are seeing something that could be better. The point of this is to have a base structure that we can then iterate without breaking anything.
This classes should not echo or print ANYTHING AT ALL.
Now that we have most of the logic contained and organized (and maybe modernized and tested!), we should start moving every embed HTML there is into a view, and maybe use a Response object. This should handle all the responses so it can be controlled.
For the time being, I wouldn't include any kind of view templating capability. The whole point of this exercise should be to leave everything as it is but better prepared for the future.
One of the few things left in the scripts around should be the Action Logic, this can now be extracted into controllers that interact with the views through the Response object we created before. This should make it much easier to refactor views in the future.
Maybe there are still some includes left embed in the logic that we couldn't remove before, or we didn't have the structure to remove them easy enough and "left them for later"... now should be the time.
The way to do this should be thought in a case to case basis.. sometimes the logic it represents is only used in one place and can just be refactored into that class. Sometimes it is used all around and needs to be refactored into another class that gets then used all around.
This is an "easy but delicate" change. As this project is in production already (and maybe not only apollo will be using it at that point), this needs to be an advertised change and well thought out.
This entails the changing of the base "file structure" of the project, as most frameworks do nowadays, to separate whatever is going to be publicly accessible and private. Symfony 4 has assets, bin, config, public, src, templates, tests, translations, var and vendor. We also have as an example Laravel with app, bootstrap, config, database, public, resources, routes, storage and tests.
I'm much more partidary of the Laravel structure, but that's just me.
I would take this time to organize some of the front end stuff too, maybe not port to a build system out of the gate, but get everything in place to be able to use webpack and all that.
This is another delicate one. This is the time to create a front controller that allows us to have a router and not have the site be only "linked pages". At this point we need to implement the 404 structure, the Router and reconfigure the server (if needed) to use mod_rewrite or whatever is needed to implement the url scheme of things.
At this points we should have "page scripts" when we had scripts before, that use the Front Controller and Router to publish a view. So we have a nearly completed modern structure.
At the last step, trying to have everything with as little change as possible, we usually end up with a buch of duplicated logic in every page script, as most of the page scripts will do only one thing, and that is link a route with probably a controller that prints a view.
I suggest using a bare almost custom DI container, as the book says (and even suggests some code to use) and keep iterating through changes.
This DI Container (that can be swapped later for a a composer package that can improve things a bit) is crucial to simplify routing and the loading of every service the site uses. Also, this will open some doors to easy refactoring in the future of custom code into packages.
At this point the code base should be cleaner and more modern, now the work of simplifying and iterating over stuff begins. At this point, if you have been cleaning up some stuff, you could have seen things that could be easily improved, and now would be the time to discuss and implement those changes.
Also, the conversation of migrating from some custom code to some composer packages can start. Like using some sort of DB Layer, the DI Container, the routing, etc.
Also should be the time to rewrite the views to use any templating engine if that's something that could be useful.
Me? I was asking for the plans of the people that are behind Apollo, just so I can get organized. Maybe @itismadness can chip in?
If I had to do this myself, as I said, I have some good experiences in the past following the "Modernizing Legacy Applications in PHP" book. To kind of summarize the steps (bear in mind it is going to be long... it is a summary of the book steps):
I would start by deciding the new coding standards (my vote is for PSR2) and setting up a CONTRIBUTING.MD file. This file should explain the goals, and help newcomers get started with the contributing process.
This entails moving all the classes into the app folder and renaming them according to the desired namespace. Applicant class -> app\Applicant.php -> Gazelle\Applicant
This should be done one by one, checking afterward that everything still works as expected. It can entail a bit more than just moving. Making sure the classes inside can be reached (correct namespaces), and refactoring every use of that class so it uses the namespaced version of it, and not the old one. I'm actually doing that on the develop branch of my fork.
Refactor all globals to use Dependency Injection. This entails moving all the global vars to be parameters into class constructors, also all the superglobals ($_POST, $_SESSION, $_SERVER) should be refactored into a class that gets shared by the application. Like a Request class.
Extract "one-time creation" to dependency injection. This entails checking the classes for any new keyword, if what we are instantiating is a new Exception or something from the SPL, we may want to just leave it like that, but if it is something that should be moved into a Factory, do it.
I wouldn't write Unit tests at this point, but I would spend the most time possible writing characterization and / functional tests. This tests would test the functionality of the website as a whole. Maybe check out and evaluate Codeception.
I would take this time to also set up some automated things on the github, like travis / style-ci. After this step, theoretically, should be easier to see if you have broken anything while refactoring...
This step creates classes (Gateways) that deal only with the getting information out of the database. I tend to think about them, even if I'm mostly wrong, as Database Models. These are the main "forces" writing and reading from the database.
This should be where the real work begins... This entails moving all of the logic that is on those generic classes that have moved around in the past steps, and that is mostly associated and around the Gateway classes, we created on the last step, into its own class.
At the same time this is going on, the logic should be simplified and maybe if applies, modernized to a php7 code base. At this point, I WOULDN'T change any of the logic. Even if we are seeing something that could be better. The point of this is to have a base structure that we can then iterate without breaking anything.
This classes should not echo or print ANYTHING AT ALL.
Now that we have most of the logic contained and organized (and maybe modernized and tested!), we should start moving every embed HTML there is into a view, and maybe use a Response object. This should handle all the responses so it can be controlled.
For the time being, I wouldn't include any kind of view templating capability. The whole point of this exercise should be to leave everything as it is but better prepared for the future.
One of the few things left in the scripts around should be the Action Logic, this can now be extracted into controllers that interact with the views through the Response object we created before. This should make it much easier to refactor views in the future.
Maybe there are still some includes left embed in the logic that we couldn't remove before, or we didn't have the structure to remove them easy enough and "left them for later"... now should be the time.
The way to do this should be thought in a case to case basis.. sometimes the logic it represents is only used in one place and can just be refactored into that class. Sometimes it is used all around and needs to be refactored into another class that gets then used all around.
This is an "easy but delicate" change. As this project is in production already (and maybe not only apollo will be using it at that point), this needs to be an advertised change and well thought out.
This entails the changing of the base "file structure" of the project, as most frameworks do nowadays, to separate whatever is going to be publicly accessible and private. Symfony 4 has assets, bin, config, public, src, templates, tests, translations, var and vendor. We also have as an example Laravel with app, bootstrap, config, database, public, resources, routes, storage and tests.
I'm much more partidary of the Laravel structure, but that's just me.
I would take this time to organize some of the front end stuff too, maybe not port to a build system out of the gate, but get everything in place to be able to use webpack and all that.
This is another delicate one. This is the time to create a front controller that allows us to have a router and not have the site be only "linked pages". At this point we need to implement the 404 structure, the Router and reconfigure the server (if needed) to use mod_rewrite or whatever is needed to implement the url scheme of things.
At this points we should have "page scripts" when we had scripts before, that use the Front Controller and Router to publish a view. So we have a nearly completed modern structure.
At the last step, trying to have everything with as little change as possible, we usually end up with a buch of duplicated logic in every page script, as most of the page scripts will do only one thing, and that is link a route with probably a controller that prints a view.
I suggest using a bare almost custom DI container, as the book says (and even suggests some code to use) and keep iterating through changes.
This DI Container (that can be swapped later for a a composer package that can improve things a bit) is crucial to simplify routing and the loading of every service the site uses. Also, this will open some doors to easy refactoring in the future of custom code into packages.
At this point the code base should be cleaner and more modern, now the work of simplifying and iterating over stuff begins. At this point, if you have been cleaning up some stuff, you could have seen things that could be easily improved, and now would be the time to discuss and implement those changes.
Also, the conversation of migrating from some custom code to some composer packages can start. Like using some sort of DB Layer, the DI Container, the routing, etc.
Also should be the time to rewrite the views to use any templating engine if that's something that could be useful.
itismadness
commented
6 years ago I agree that we should definitely write up a CONTRIBUTING.md that lays out style guides (especially as the style in app/ is going to be different than the legacy code to be more inline with how everyone names variables, classes, etc.) which also would lay out the high level dream for how I'd like to approach development (as I disagree with the idea that a good initial step would be to move everything to PSR-4 namespaces as that gains us minimal ground, and also makes it harder to see "what is still old crufty code that we need to rewrite" or that tests should be left till later as realistically that just means that the tests will almost never be written).
TJSoler
commented
6 years ago Makes sense that, being a "non-main work project" where things could be left hanging at any time, crufty code is not being "actively prosecuted with prejudice". And with that assumption, it also makes sense that you'll want to have a physical separation between what is new and what is not.
I think the book leaves testing to that point only because is somewhat easier to achieve if you don't have globals, and the classes are already well formed and don't need a bunch of includes all around. But thinking of the amount of work that is needed to achieve that point, is not that much, so it is still "write tests before touching any of the logic stuff". But I digress.
I guess I can wait to have a CONTRIBUTING before jumping over stuff, or maybe I should apply inside apollo. I'm up for setting up some functional testing even before touching anything else.
I'm thinking I might be able of sinking a few hours a week into the refactor of this project.
Are there any established short / medium / long term plans?
I set up the vagrant project locally and have been doing some tests to see if it would be viable to move to a PSR4 autoloading scheme, and seems to be decently easy for the time being.
If I had to start this, I would follow http://mlaphp.com/ something similar to that book. I did that in some projects before and turned out great.
https://github.com/TJSoler/Gazelle/tree/develop you can see the kinds of changes I'm talking about on that branch.