Skip to content

Conversation

JasonTheAdams
Copy link
Member

@JasonTheAdams JasonTheAdams commented Mar 8, 2025

Many of the properties and methods of the Model class are instance properties for no particular reason. The Model::$properties property, for example, is an instance property. But it's really only meant to be defined by the class and then consistent across all subclass instances. In fact, making the properties dynamic on a per-instance basis is an anti-pattern to the model itself, as it's no longer a clearly defined set of data.

The side-effect of these being instance bound is that an instance is required in order to do basic things such as know a model's properties, check if a property is valid, and so forth.

This PR switches a number of properties and methods to be static. Basically those that can be. It updates the interfaces and the Model class itself.

Accessing a static method from an instance (e.g. $model->hasProperty() is supported in PHP and backwards compatible, but the interfaces and property/method overloading obviously aren't. As such, this is a major change and slated for a 2.0.0 release.

@JasonTheAdams JasonTheAdams changed the title Refactor: Reasonable model methods and properties are made static 2.0 — Refactor: Reasonable model methods and properties are made static Apr 9, 2025
@JasonTheAdams JasonTheAdams marked this pull request as ready for review April 9, 2025 22:10
@JasonTheAdams JasonTheAdams added this to the 2.0 milestone Apr 9, 2025
@JasonTheAdams JasonTheAdams requested a review from borkweb April 11, 2025 15:11
@JasonTheAdams
Copy link
Member Author

@borkweb Mind reviewing this, again? I think this is pretty safe to merge and will knock out a dependent branch of the other PRs.

@JasonTheAdams
Copy link
Member Author

I'm going to merge this since it's been discussed and it will help clean up my other PRs.

@JasonTheAdams JasonTheAdams merged commit 4ccd424 into release/2.0.0 Apr 12, 2025
1 check passed
@JasonTheAdams JasonTheAdams deleted the refactor/static-methods-properties branch April 12, 2025 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants