Skip to content

First pass at object factory#1218

Closed
joshlevinson wants to merge 4 commits intotimber:masterfrom
joshlevinson:feature/object-factory
Closed

First pass at object factory#1218
joshlevinson wants to merge 4 commits intotimber:masterfrom
joshlevinson:feature/object-factory

Conversation

@joshlevinson
Copy link
Copy Markdown

Issue

Begins to solve the problem of a lack of control over which classes Timber uses to instantiate WordPress objects.

Solution

This introduces a factory for creating Timber posts and terms.
Abstracts the class mapping functionality previously available only to posts

Impact

This change doesn't itself introduce back compat issues; future changes/usage might.

Usage

Changes how Timber WP objects are instantiated. These should be created with the appropriate Factory, rather than directly instantiating the Timber class objects.

Considerations

This solution doesn't introduce a factory for instantiating every Timber object. That may or may not be desirable. I'm up for discussion 🙂

Testing

Yet to be done!

Implements a factory for getting Post and Term objects
Abstracts the class mapping functionality previously available only to posts
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 21, 2016

Current coverage is 92.77% (diff: 0.00%)

Merging #1218 into master will decrease coverage by 2.88%

@@             master      #1218   diff @@
==========================================
  Files            43         47     +4   
  Lines          3181       3280    +99   
  Methods         424        436    +12   
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           3043       3043          
- Misses          138        237    +99   
  Partials          0          0          

Powered by Codecov. Last update 3a1deee...cddb3b2

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 21, 2016

Coverage Status

Coverage decreased (-2.9%) to 92.368% when pulling d772ded on joshlevinson:feature/object-factory into e89207b on timber:master.

@joshlevinson
Copy link
Copy Markdown
Author

joshlevinson commented Oct 21, 2016

Note that the travis-run unit tests appear to have failed due to an http/svn-related issue. The test passed on all but one environment. I'm running PHP7 locally, but didn't make use of any features that are specific to > 5.4.

@jarednova
Copy link
Copy Markdown
Member

@joshlevinson I'm about to get to bed, but I'm super excited about what I see. This seems to really clean things up (I didn't realize how much of the Post and Term class could really be delegated to the Factory to handle the BS like check_post_id). I hate to fully communicate my ignorance, but I want to make sure I fully educate myself on the use of factories as a programming concept before I get to deep. Is there any recommended 📖 that you have to help get me better up-to-speed?

@joshlevinson
Copy link
Copy Markdown
Author

joshlevinson commented Oct 24, 2016

I don't have any good books handy to recommend; most of my reading has been online.
You can check out these links though:

joshlevinson added a commit to joshlevinson/timber that referenced this pull request Oct 24, 2016
This style provides different factories for Posts vs Terms, and allows direct instantiation of the factory vs strictly a static method.
This is more future proof than the purely static method of timber#1218, and opens the door for stubs and better testing.
@acobster
Copy link
Copy Markdown
Collaborator

@jarednova @gchtr @joshlevinson I would be interested in picking this up again if there's any chance of getting a feature like this into 2.x. Especially with regard to being able to (somehow) specify a factory to use when calling get_all and friends.

Example use-case:

  • say I have custom Page, Post, and CustomPost classes that all extend Timber\Post
  • I want end-users to be able to search all of them, across post types
  • I want to display their search results polymorphically in a Twig template that calls {{ post.get_search_excerpt }}

@jarednova
Copy link
Copy Markdown
Member

Doing some cleanup with @acobster deep into Factories over on this branch:

https://github.com/timber/timber/tree/2.x-factories

... closing @joshlevinson's greatly appreciated PR here. Even if the exact code won't live on, the spirit will. Thanks for getting us started down this path!

@jarednova jarednova closed this Jan 8, 2020
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.

6 participants