Nice, I like this idea.

Would it be worth creating a TimberCollection class that extends ArrayObject and then get the Iterator in that way? That way, we can add some helpful collection methods too and provide the option of retrieving as a collection. (like what is in laravel or backbone)

Awesome! I'm just getting back from my vacation but excited to tear into this. What are the risks like with backwards compatibility? Or is it really limited to the specific test situation you mentioned above where I'm using the array offset?

@bryanaka Do you mean creating a TimberCollection class that serves as a wrapper for either a WP Query collection and / or array-of-posts collection? If this is a child of ArrayObject, the risk is that it would allow array-foo on the object, even if it's an iterator over WP Query. Perhaps it could magically switch to an array of posts if the user performs array manipulation on the object..

@jarednova The only risk with backwards compat (I can see) is with very specific array-like operations on the posts, eg. array_reverse, or array_splice, etc. stuff. Admittedly, one of the handy things of getting an array of posts from Timber::get_posts was that you could do this when dealing with complex queries. However, this is quite detached from WP, the first implication being that template tags like get_the_ID() stop working.

Here's an idea: add a toArray() method to the iterators, that returns the arrays like Timber::get_posts() used to..? This way, whenever you want to really play with the post data, you could grab the array and use it like that.

Another idea: splitting things up into two separate methods - Timber::get_posts() to get posts in an array (current behaviour) and Timber::query_posts() to get the posts in one of the iterators. This (sorta) makes sense within WP as get_posts() also returns an array of posts. Timber::query_posts() would be the equivalent of $q = new WP_Query( $args );.

The last solution would ensure backwards compat by simply offering the iterator as just a new alternative.

Just pushed the changes I described in comment above. The solution is now fully backwards compatible, as Timber::get_posts simply returns an array of TimberPosts, like it used to. It goes via Timber::query_posts, which creates either a TimberPostsCollection or a TimberQueryIterator. Both implement a get_posts method, which returns the 'normal' array of posts.

I put some more thought into @bryanaka's suggestion and turned TimberPostsIterator into TimberPostsCollection, that extends ArrayObject and uses TimberPostsIterator for its iterator. This could be extended to also work with WP Query (by taking WP_Query::posts out of the object, like TimberQueryIterator::get_posts currently does), but I'm not sure if this is better or not. It makes sure you're always working with the same object, which could get some nice extra utility methods, but it would be less closely integrated with the ordinary WP Loop (not necessarily bad..). Also, even ArrayObjects don't work with a lot of array functions (the reason we're doing PHP, right?), so the added benefits may not be huge. Something to think about still..

@mgmartel Yeah, unfortunately almost none of the native array functions work on ArrayObject. Which is really stupid in my opinion.

Here is my first iteration of dealing with that limitation, with some added sugar (ideas from rails and backbone):
https://github.com/bryanaka/lumberjack/blob/master/lib/lumberjack/collection.php
https://github.com/bryanaka/lumberjack/blob/master/test/lumberjack/collection_test.php

@bryanaka That's good stuff! Especially since an ArrayObject still comes with the getArrayCopy method, for if you're really desperate for native array manipulation.

Using your collection object + the query iterator = a full fledged WP loop in Timber with the possibility of manipulating the posts array any way you want. (=freedom )

There are still a few unit tests I want to write and pass, but when I get some free time, ill submit a PR.

I will mention though that this is a good opportunity to think about supporting PSR-0 naming conventions. This would break stuff, but should be done before 1.0 if it is still favored.
—
Bryan Robles
Full-Stack Web Developer
http://bryanrobl.es

On Sat, Jan 11, 2014 at 11:57 PM, Mike notifications@github.com wrote:

@bryanaka That's good stuff! Especially since an ArrayObject still comes with the getArrayCopy method, for if you're really desperate for native array manipulation.

Using your collection object + the query iterator = a full fledged WP loop in Timber with the possibility of manipulating the posts array any way you want. (=freedom )

Reply to this email directly or view it on GitHub:
#138 (comment)

@mgmartel I am pretty slammed with work, and transitioning to a new position soon. I don't know when I would actually get around to making the PR. If you have time, go ahead and use my code as needed.

@bryanaka No rest for the wicked! I'm also buried in work at the moment. ...but mainly with a Timber-based project, so I'll try to make an excuse to do some more fiddling soon.

Is this something that'd be good for me to take a look at? or should I wait until @mgmartel has a little more fiddling time?

@jarednova definitely.

Since I won't be working with wordpress primarily at my new position (on to Ember.js work, though I'll be moonlighting with wp), it would be good to get the viewpoint from other people whether the kind of functionality I am providing in the collection class is actually providing value when working with wordpress, or a case of over-engineering.

@jarednova Absolutely! The pull request as it stands works as it stands, definitely now that Timber::get_posts is fully backwards compatible through Timber::query_posts. The added benefit of @bryanaka's Collection is that you wouldn't have to extract the array from the TimberCollection object to do array-foo.

I'd be very happy to see such functionality, because Timber would then inherit all goodness from WP, and even go beyond that by making the post results as malleable as an array.

Then it'll be good to see where in example code and the starter theme things can be improved by using Timber::query_posts instead of Timber::get_posts (and perhaps also find what the cases are where one would still want to use get_posts instead of query_posts).

@jarednova Just had a look to see if I could do a quick and rough merge in of @bryanaka's Collection class, and stumbled on some little mistakes in the PR. I'll try and piece together a quick sketch now, should push that within the hour.

Of course, a lot came up suddenly. I just pushed the first quick pass of integrating @bryanaka's Collection class with this PR to this branch:
https://github.com/mgmartel/timber/tree/timber-collection

Yo @mgmartel! It's been a crazy month wrapping up some projects but I finally had a chance to review the Pull Request. In my tests (both automated and touring some development sites) everything's looking good. I was hoping you could help me with some questions...

• Tests Is there an example of a test that we could write showing how the previous code would fail where this would succeed? For me this is also about helping me understand the new usage this unlocks and how to convey it in the next WP.org release
• Next version I've been thinking alot about how Timber has grown over the last 9 months (from Twig to TimberPost to Pagination to Collections and Iterators). I'm starting to feel like I need to take a pause and do some planning work to plan-out a full cohesive architecture to comprehensively support all WP objects and collections. Based on your schedule right now are you available to lend some brain power towards some planning-out the roadmap?
• Merge now? Merge later? I've pushed this pretty hard against some projects I'm working on and haven't found any issues. Based on where you are, is this ready to be pulled into master? Or should I wait until you have a full chance to merge and verify against @bryanaka's Collection class?

Thanks!

Hi @jarednova! I'll be finishing up on a project that uses Timber extensively the coming month, so I'll be busy, but will make sure this PR is nice and polished before that goes into production. First thing I'll do is write some tests (and also think about what other practical added benefits of using the iterator are). I'll do that some time in the coming week.

After that I'll play around with the TimberCollection, that uses @bryanaka's Collection class and see how it performs, if there's any unexplored stuff there, etc.

I'm not sure what the best strategy is merge-wise: the PR as it stands is (as far as I can see) quite robust and backward-compatible. At the same time, it may not be worth it merging it already, as it may inhibit further development (we don't want to change and then change again on the master branch). I'm leaning towards merge later, at least until we have more clarity on what's happening with Collections etc.

Re:Next version. Hell yes. I'm wrapping up projects this month, so I'm expecting a lot of unexpected work, but I'd be happy to brainstorm and contribute to the next version. I'll start listing out some thoughts and ideas next week too. Shall we open a new issue for discussion?

Hey @mgmartel I opened a thread here for discussion, and also a wiki document we can refine towards a final set of goals and planning:

https://github.com/jarednova/timber/wiki/Roadmap

I'd love to hear your thoughts about a) the values and goals b) architecture. I feel like you've got a leg-up on me when it comes to some of the more advanced PHP stuff that we could take advantage of to make sure we build upon a solid foundation.

Hey @jarednova . 'This week' turned out to be a lie! We pushed my deadline a week closer (because Hofstadter's Law is for wusses ). Anyway, have been working with Timber a lot so got some smaller fixes in the pipeline.

Just quickly wrote a test to show how the iterator allows the use of template tags like 'get_the_title' to work (not much of an accomplishment, but hey ;) ). Will spend more time on it after this week, when my current project is online.

Yo @mgmartel thanks for the update. It helps me get a better understanding of the potential upsides this approach can be used for. I've also been running your branch of my local for the last week and haven't run into any backwards compatibility issues with the sites I'm developing. So that we don't lose momentum on this I'm thinking about merging this into the dev branch and then build upon it from there -- how's that plan work for you?

Hi @jarednova. I've been switching around the WP plugin repo version and the Iterator branch and haven't found any incompatibilities either. I'm all for merging at this stage. I have some changes that currently live in branch limbo because I implemented them for a project on a branch off the use-iterator branch and it would be nice to start bringing everything together again.

What is your policy around the dev branch? This PR is on the dev branch, but it seems to be behind master. If you say go, I'll take the use-iterators branch (with collections?) and rebase it on dev or master.

Hi @jarednova, I'm trying to rebase this branch on master, but am running into problems. Before deleting the dev branch, github.com:mgmartel/timber was merged into into mgmartel-use-iterators (in d6e6b83). So the commits from this branch are a part of master, but not effective. A rebase (or merge) will therefore do zip.

Not sure how to get this branch up and running now. Will the changes from these commits become effective again when you merge mgmartel-use-iterators into master? Or should we replay the changes on top of the current master?

Based on my Git understanding, I think we should do something to replay. I'm going to call-in one of my officemates who's better w Git to help me out. I'll get his input and then get right back to you....