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

Already on GitHub? Sign in to your account

Add ability to return Custom Object Collections from model query #314

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

theoreticaLee commented Oct 25, 2012

Added a behavior to create XXXObjectCollection.php class per table so we can have specific methods on each object collection depending on the domain.

We talked about this briefly over the propel dev chat, but wanted to ping you'll to see if you'll like this direction before I write the tests to wrap this up.

thoughts?

Member

staabm commented Jan 9, 2013

I would love to get something like hat into the framework. 👍

another thing which came into my mind while reading the code:
couldn't you redefine all collection methods which take a Propel-Object as argument and strictly typehint them, so the php-runtime/an ide will throw errors when e.g. insert a Car into a UserCollection..

So we would generate something like

class UserCollection extends PropelCollection {
  public function add(User $user) {
    return parent::add($user);
  }
 // ...
}

when propel would generate such code for us, we would have simulated something like Generics in Java for those collections..

Member

staabm commented Jan 9, 2013

also to get this information into the IDE the methods inside BaseXXXQuery classes need to be adjusted when generating the sources

e.g. instead of
* @method array findByPublishDate(string $publish_date) Return Article objects filtered by the publish_date column

something like
* @method ArticleCollection findByPublishDate(string $publish_date) Return Article objects filtered by the publish_date column
would be great!

Contributor

theoreticaLee commented Jan 11, 2013

@staabm Sounds good to me. I'm game if this pull request gets some notice from the core team.

@jaugustin jaugustin commented on the diff Jan 11, 2013

src/Propel/Runtime/Formatter/ObjectFormatter.php
@@ -51,9 +56,30 @@ public function format(StatementInterface $stmt)
return $collection;
}
+ public function setCollectionClassName($collectionClassName)
+ {
+ $this->collectionClassName = $collectionClassName;
+
+ if (false === class_exists($this->collectionClassName)) {
+ throw new RuntimeException(sprintf(
+ "Could not find %s class. Please ensure you rebuild the model and loaded the class.",
+ $this->collectionClassName)
+ );
+ }
+
+ if (false === is_subclass_of(new $this->collectionClassName(), "\Propel\Runtime\Collection\ObjectCollection")) {
@jaugustin

jaugustin Jan 11, 2013

Member

why not using instanceof ?

@theoreticaLee

theoreticaLee Jan 11, 2013

Contributor

Good idea, instanceof fits better with how propel usually checks for this.

@gharlan

gharlan Apr 10, 2013

Contributor

But with is_subclass_of you could pass the plain class name (without object creation):

if (false === is_subclass_of($this->collectionClassName(), "\Propel\Runtime\Collection\ObjectCollection")) {
@staabm

staabm Apr 10, 2013

Member

@gharlan good point. I think we should stay with is_subclass_of but omit object creation..

Member

jaugustin commented Jan 11, 2013

@staabm I think we should also add docbloc in the <Custom>ObjectCollection for all methods that return object

@theoreticaLee is it possible to add some tests ?

Member

staabm commented Jan 11, 2013

yes, this would be great.

we need to check first, in case we decide to redefine all collection methods and adding a typehint, if this will not trigger strict warnings, because the functions' signature of the subclass doesn't match the signature of the parent class... I am not sure how php handles this case..

Contributor

theoreticaLee commented Jan 11, 2013

@jaugustin yeah, I'll go ahead and write some tests now I have gotten some feedback.

thanks guys

Owner

willdurand commented Jan 11, 2013

thank you for the effort @theoreticaLee, really appreciated!

Member

staabm commented Jan 12, 2013

@theoreticaLee regarding the generation of phpdoc comments, I had a look into the Source of the SourceBuilders ... Unfortunately there is no hook into the comment generation atm.

@willdurand what do you think about defining a hook method for phpdoc generation?

@staabm staabm commented on the diff Apr 10, 2013

...CustomObjectCollection/AddObjectCollectionBuilder.php
+ *
+ * @license MIT License
+ */
+
+namespace Propel\Generator\Behavior\CustomObjectCollection;
+
+use Propel\Generator\Builder\Om\AbstractOMBuilder;
+
+/**
+ * Adds an object collection class unique to the class
+ *
+ * @author Lee Leathers
+ */
+class AddObjectCollectionBuilder extends AbstractOMBuilder
+{
+ public $overwrite = false;
@staabm

staabm Apr 10, 2013

Member

unused property?

Member

staabm commented Apr 10, 2013

would love to see some tests. phpdoc generation should be handled by a separate PR to get this feature into the codebase.

Owner

willdurand commented May 11, 2013

This is not critical, but it is a nice feature. Let's ship it for the beta milestone.

Member

staabm commented May 11, 2013

@willdurand maybe we should label everything which we think should be part of the beta, so everyone knows in whats needs to be done next

Owner

willdurand commented May 12, 2013

Almost all alpha issues are tagged. We can leverage the milestone feature to identify beta issues. WDYT?

@willdurand willdurand closed this Mar 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment