View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 5838 | Composr | core | public | 2024-08-04 16:21 | 2024-08-04 18:57 |
| Reporter | Chris Graham | Assigned To | Guest | ||
| Priority | normal | Severity | feature | ||
| Status | new | Resolution | open | ||
| Summary | 5838: Modernize the codebase | ||||
| Description | A big chunk of our codebase is very procedural. This is part-historic (we started on PHP4 which had poor OOP), part-technical-debt (very time consuming to change, and not the biggest priority), and part intentional to keep things highly-performant. We have a lot of testing, but it is more integration tests than unit tests. We would be able to do much better unit testing if we moved to use classes for most of the files under the sources directory. Generally we should have a stricter model/controller split. Models should be the only thing interfacing with the database. And generally the interaction of classes should be mediated by either dependency injection or at least abstracting instantiation into standalone methods that could be overridden in mocks. This allows us to do unit test mocking, e.g. to substitute fake database responses so that tests don't rely on a real database with real data. We should not be using globals or static inside functions; if we need caching we can use object properties. We should make use of PHP autoloading, which has been mature for a long time. We should convert all PHP error handling to exceptions. Generative AI could probably help a lot writing a lot of new unit tests with mocking. If we are to make future big overhauls in the codebase, like moving to TypeScript, we will need good testing. The most performance-critical code should probably remain procedural, so we don't have to do lots of dereferences going on in tight loops. But that's probably not a lot of code. The performance-led splits we have now, e.g. galleries vs galleries2 could be preserved in OOP too. My argument against excessive-OOP in the Code Book can be altered to be a bit more nuanced. We don't need to go to the extent where we suddenly have a million tiny classes with bikeshedding over where exactly responsibilities lie. This is mainly about allowing better testing given how big the platform has become and how flexible the code is sometimes required to be. YAGNI (You aren't gonna need it) should be used as a design strategy, as opposed to over-engineering. | ||||
| Tags | Roadmap: Over the horizon | ||||
| Attach Tags | |||||
| Time estimation (hours) | 1000 | ||||
| Sponsorship open | |||||
| related to | 355 | Closed | Chris Graham | MVC rewrite |
|
|
Side note, there is a new v11 function in sources/developer_tools.php called make_dummy_db_row. This generates random data based on the schematics. While it does actually insert into the database, this could easily be modified to instead return the dummy db row without actually inserting it. This was mainly made for my new stats test |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2024-08-04 16:21 | Chris Graham | New Issue | |
| 2024-08-04 16:21 | Chris Graham | Tag Attached: Roadmap: Over the horizon | |
| 2024-08-04 16:21 | Chris Graham | Category | General / Uncategorised => core |
| 2024-08-04 17:34 | PDStig | Note Added: 0009071 | |
| 2024-08-04 17:35 | PDStig | Note Edited: 0009071 | |
| 2024-08-04 18:57 | Chris Graham | Relationship added | related to 355 |
