WBCE / WBCE_CMS

Core package of WBCE CMS. This package includes the core and the default addons. Visit https://wbce.org (DE) or https://wbce-cms.org (EN) to learn more or to join the WBCE CMS community.
https://wbce-cms.org
GNU General Public License v2.0
31 stars 22 forks source link

Database Class: Make flexible - Multiple database, more flexible, consistent #551

Open Forshock opened 9 months ago

Forshock commented 9 months ago

Moved to WBCE from WB, and had a few suggestions for the Database class. I wont necessarily provide code, I know enough to get the job done well enough, but others may have better understanding than I. I ran into a scenario where I needed to use a DB from different server and dont want to have multiple frameworks that all need maintained.

  1. Multiple/Other connections - Remove the hard-coded defines (DB_HOST, DB_USERNAME, DB_PASSWORD, DB_NAME, DB_PORT) and allow for manual specification. Easiest method may be to use arguments for the connect function? connect()
  2. get_one/get_array - Dissimilar methods of query. get_one uses the mysqli_query function, while the getarray passes through the mysql class. Seperating the mysqli functions from the database class could allow for future versions with database flexibility (PDO, Oracle, etc.) . (See next)
  3. Database Extensions (Inheritance) - Moving the specific database interfaces to the inheritance classes to allow for multiple database vendors or interconnection from modules. There has to be project already out there right?

Keep up the good work.

instantflorian commented 9 months ago

Thank you for the suggestion, but this sounds like a rather huge effort.

WebDesignWorx commented 9 months ago

These are good suggestions but at the moment we concentrate on another areas of the CMS and the Database Class won't have (m)any changes in the WBCE 1.x version. The only exception would be if working code is provided via a pullRequest for revision. We are open and willing to consider suggestions and implement improvements, but at the time being we are not in the position to do everything ourselves.

mrbaseman commented 9 months ago

There have been various attempts to change the database class. Some have been implemented, some others got stuck during implementation or already in the discussion phase. PDO and making use of prepared statements are examples that I have seen here or in the forum every now and then.

The problem with the database class is, that it is used throughout the core code and also in all modules, so one has to stick to the current API while doing a rewrite of the lower level, and that's quite tricky. You can add new features with optional arguments or new methods, but even if this is implemented and working, it presumably takes ages to get adopted in the modules, just like the transition from phplib templates towards twig ;-)

Don't get me wrong, I'm not arguing against a modernization of the code. I just wanted to add my 2ct that especially the database class is a quite difficult area.

Forshock commented 9 months ago

I do understand the hesitation, especially with so many modules having their own database interaction.