Habe you checked that we safely can replace the initial null values with empty arrays? What if the null values were intentional and are checked somewhere else? In my opinion code review is not a sufficient test method, especially not if people only review the changes but not other places in the code where the changed code is used. I think it always needs real testing if something is broken or not, and not just code review without any further instructions.
Hello @Hackwar, thank you for your pull request. It is a very good one.
This changes can break backwards compatibility, so to be on the safe side I wold earliest merge it into Joomla 5. Can you rebase it so people can start testing?
Thank you very much for your answer, really appreciated. As you change some default values on variables in the Factory class, I fear this might have some side effects into the wild.
Yes, the default is changed, however the factory itself only accesses that as an array and without initialising it first, it would throw a notice. Any code currently accessing this would have to check on array AND if the array-entry is set anyway. There is no situation where this could create issues. The only thing I could think of would be people resetting the cache again by setting this back to null instead of an array and that would throw a notice. But it would do that in the current code too.
Habe you checked that we safely can replace the initial null values with empty arrays? What if the null values were intentional and are checked somewhere else? In my opinion code review is not a sufficient test method, especially not if people only review the changes but not other places in the code where the changed code is used. I think it always needs real testing if something is broken or not, and not just code review without any further instructions.