OOP is our foundation - no global functions and vars anymore.
I want everybody to vote on this one and/or comment if there are any objections or questions.
To me that is obvious that in 2018 OOP is not something abstract or something we need to explain and/or discuss. All modern php frameworks are OOP, MVC and PSR friendly.
I don't want to push us to be PSR extremists. But I think that would be very good for the project if we can get rid of all our global functions and move the functionality to related classes.
For example:
c() function should be a method of some Configuration class
t() function should be a method of some Translation class
dbencode(); dbdecode() functions should be a method of some Database or Model class
val() function should not exist at all
...etc
Lets agree to not introduce any new global function or variable?
Comments
-
I should say that this is the eventual goal, but we need to work on having the facilities to replace the global functions first. I'd like to see concrete recipes on replacing global functions with classes. Here are some of my thoughts:
- We have dependency injection, but it can make class constructors unwieldy. What are our solutions for this?
- What are the best ways to make such facilities available to addons?
- The view layer needs translation at least. How/where is it provided?
- What about traits?
- Translation is a difficult one to work with here. We generate user-facing strings everywhere and removing the global function will be extremely tedious without a decent facility to do so.
That's the type of thing I'm talking about. To me, we need to have solutions in place before we make the rules.
I also don't 100% agree that there is no place for global functions anywhere. Otherwise there would be no need to use built in PHP functions at all. In our case, before the
??operator,val()served a useful purpose for example. Furthermore, most any templating library has its own flavor of template functions that work much like global functions.Global functions that reach into config or something like that are certainly problematic. Those are the ones we should aim to get rid of as a highest priority. We just need a way to get rid of them for the more difficult edge cases.
0 -
@Todd There are too many topics you are trying to discuss at once :-) And that is why, I am trying to keep general ideas but not strict standards yet. If we can agree to not introduce any new global function - that would be a big step forward :-) I have all answers to any topic you just mentioned. But as you can suspect - we can have different points of view on many of them. So lets start it one by one.
Feel free to set new discussion about any of problems you just raised and we will discuss it one by one to get to some recommendations team can follow in future.
0 -
We can certainly agree not to add global functions moving forward.
1 -
When you say "no global functions" do you mean "no functions"? Would namespaced functions be acceptable or would they be required to be methods in a class, even if that class was only serving as a mock namespace with no state?
0 -
My understanding after discussing the issue with him last week was that namespaced functions would be acceptable. The root issue is loading every function into memory on every request regardless of whether they will be used.
2 -
If memory is the issue then research into how APC caches code is necessary. It is my understanding that code is cached in shared memory and not per-request memory. And although the autoloader is efficient, it's not as efficient as including known files right away.
0 -
@Ryan @Linc There are few things I want to clarify about the whole initiative to "deprecate everything" :-) and to make a "cookbook". I want to move slow but fast. On one hand I hope we can get rid of legacy code completely. On other hand I see that is mostly not possible to achieve 100%. That means anyway we will have some legacy no matter how hard we will push the initiative and how much efforts we will apply.
Knowing that I have a big warning in my mind: I don't want us to create "new legacy" code.
So coming back to your question: yes, no global functions means few things.
- 1st and main is: no more new functions (because I've checked them and few fresh one from this year were found)
- 2nd - yes namespace is preferable way to go. But that is not the main point. I don't want us to create set of new useless and/or meaningless classes and namespaces. I would prefer to keep functions as is until we can detect best namespace where we want to move the function. To me that is important thing that need to be explained to every developer. We need to describe how to detect best namespace if it is not clear. We can set few temporary namespaces. But at the moment I see a lot of questions even inside those global functions many of them don't follow MVC principles and don't respect OOP SOLID principles. Lets transform those functions carefully with some learning curve and "recipes" for developers and/or probably with something we can share even to open source community to follow when they distribute to Vanilla core and plugins.
- 3rd (goal) - I have feeling that many functions should not be converted into any namespace/class but need to be removed from the project. And even if we can easily agree on some of them we still have no clear instructions and/or procedure how to do that in reality. I see that there were some initiatives in the past. We already tried to do that (that is why `deprecated()` function exists I guess). But it is not clear if we maintain or push that initiative. I don't want us to set new initiative about same topic every year :-) Lets try to set some doc about that. Lets provide some example of how we can deprecate function from start to end.
As I already replied to Todd, lets agree on something small and simple to start. All the rest - lets open new and new topics/discussion/post under "cookbook" initiative and move forward step by step.
1 -
This is what we have in composer.json
```
"files": [
"library/core/functions.error.php",
"library/core/functions.general.php",
"library/core/functions.compatibility.php",
"library/core/functions.deprecated.php"
],
```
Can you give me any request example when all this 4 files will not be called? all functions will not go into memory? IMHO there is a big difference between namespace called and autoloaded through composer, and these "files".
Here is another example:
in our bootstrap.php
```
require_once PATH_LIBRARY_CORE.'/functions.validation.php';
require_once PATH_LIBRARY_CORE.'/functions.render.php';
```
do we need this "render" functions all the time? even for api calls? I don't know. I am really just asking. To me that is obvious that either this _functions.render.php_ should be called same way from composer, or not called at all and converted into some Render class, or not exist at all and we should have some good templating library in use instead of set of "rendering" functions. And again, to me this is separate topic to discuss. Please try to open new discussion about some particular topic you want to discuss. This one is about small thing: "no more globals".
0 -
There are not requests that use all functions, but that is not my question. My question is whether or not including the functions takes more memory which requires a greater understanding of PHP internals.
You and I went through some profiling and it seemed to indicate that the memory consumption is not impacted. Certainly, I think we need to make sure we have enough op cache allocated to accept all of our source code at once.
In terms of moving things to more autoload calls, I know from profiling the addon manager that this does slow things down.
I'm okay with doing anything if it is for maintainability. However, if we are doing things for optimization we have to make sure those things are actually optimized and hence my call for research into assumptions.
1 -
I am interested!
1 -
Certainly, I think we need to make sure we have enough op cache allocated to accept all of our source code at once.
That's an excellent idea. If we haven't run an op cache profiler since the memory issues began, we should definitely do so. I've definitely run into op cache fragmentation issues that led to memory errors in the distant past. I haven't seen any since APC days, but that doesn't mean it's not possible.
0
