Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure The Package #7

Merged
merged 8 commits into from
Jan 22, 2017
Merged

Restructure The Package #7

merged 8 commits into from
Jan 22, 2017

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Jan 22, 2017

Summary of Changes

The package currently ships abstract classes that aren't largely useful on their own and forces a structure where all models are stateful. I propose to restructure things a bit.

First, I introduce two new interfaces, DatabaseModelInterface and StatefulModelInterface which define models which are aware of our database driver or hold a state. These interfaces only define getters for their data sources.

Second, I have deprecated the existing ModelInterface, models should implement the interfaces they need if desired. The setter for the state remains in this deprecated interface.

Third, I have deprecated the abstract classes. They don't provide anything useful on their own.

Fourth, I have created traits for the new interfaces which defines an implementation for the getters, a class property for the data, and setters for optional dependency injection not using a constructor.

The end result is the 2.0 package will only contain the two new interfaces and the traits.

@mbabker mbabker requested a review from wilsonge January 22, 2017 17:52
@wilsonge wilsonge merged commit 6d01efa into master Jan 22, 2017
@wilsonge wilsonge deleted the restructure branch January 22, 2017 17:53
@eddieajau
Copy link
Contributor

eddieajau commented Jan 22, 2017

For what it's worth, and with the benefit of 5 years of hindsight, I've come to find very little need for any sort of Model Interface at all. The only place I use something akin to a concrete instance are for defining data structures for my ORM (in Joomla terms, the Database\Table is your model). So I would say you could likely completely drop the package altogether.

DatabaseModelTrait is probably better living in the Database package.

These days I use service layers to access data, for example like what you'd find in Angular.

@mbabker
Copy link
Contributor Author

mbabker commented Jan 22, 2017

I don't see something like JTable ever becoming a Framework package personally. So between this and joomla-framework/view#10 it in effect makes this package 100% optional and borderline obsolete.

@eddieajau
Copy link
Contributor

Yeah, I agree. I only mentioned Database\Table to say that it's the closest equivalent to a structured model I use these days.

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.

3 participants