GibbonEdu / core

Gibbon is a flexible, open source school management platform designed to make life better for teachers, students, parents and leaders.
https://gibbonedu.org
GNU General Public License v3.0
462 stars 300 forks source link

Meta: retire old functions in functions.php #1689

Open yookoala opened 1 year ago

yookoala commented 1 year ago

Problem

There are roughly 4 types of functions in functions.php:

  1. Old style database reading functions with old school $guid and/or $connection2 (\PDO) as parameter.
  2. HTML element rendering.
  3. Utility functions for formatting (e.g. email body sanitation) or other helper functions (e.g. resolving client IP address).
  4. Shortcut for accessing core service feature (e.g. (), n(), __m()).

The first type of function cause usage of all the global $guid and $connection2, which is an anti-pattern when transiting to service container architecture.

Proposed Solution

  1. The first type should be rewritten into Gateway service method (or, in some case, rewritten as service provider).
  2. The second type should be rewritten into Form component or other Outputable class.
  3. The thrid type can be rewritten as static methods of some utility classes.
  4. The forth type can be left as-is for now.

In the rewrite, all obsoleted functions should not be removed for now. Instead, they are to be marked deprecated in their phpdocs doc-block. Modern IDE (e.g. VSCode) will identify the deprecated function calls and warn developers.

Alternatives

Leaving things as-is.

Additional Context

Functions to refactor:

SKuipers commented 1 year ago

Amazing, thanks for getting this initiative underway and documenting it so well 😃