PHPDocs for eZPersistentObject #138
Conversation
eZPersistentObject calls $this->fetch() in the constructor, so this function has to exist in all inherited classes. Perfect case for an abstract. I also thought about adding an interface, but I think that would be too much at this point.
Declared the method as protected because it is used only by eZPersistentObject::__construct(). Added return values Added error logging when required information is missing. Extracted function call from foreach statement into own line.
| * </code> | ||
| * | ||
| * @package eZKernel | ||
| * @method fetch |
There was a problem hiding this comment.
If you mean @method:
The line $row = $this->fetch( $row, false ); can be found in the constructor of eZPersistentObject, but the method eZPersistentObject::fetch() is not defined. Normally, we would solve that by defining an abstract method and by declaring the class as abstract, but we discussed that.
The @method makes sure that PHPDoc validation passes and that IDEs don't mark the missing method as an error.
| * @param array $rows | ||
| * @param string $class_name | ||
| * @param bool $asObject If true then objects will be created, if not it just returns $rows as it is. | ||
| * @return array |
There was a problem hiding this comment.
@return eZPersistentObject[]|array
| @@ -925,28 +993,33 @@ class eZPersistentObject | |||
| $cond_text = eZPersistentObject::conditionText( $conditions ); | |||
| $rows = $db->arrayQuery( "SELECT MAX($orderField) AS $orderField FROM $table $cond_text" ); | |||
| if ( count( $rows ) > 0 and isset( $rows[0][$orderField] ) ) | |||
| return $rows[0][$orderField] + 1; | |||
| return (int) ( $rows[0][$orderField] + 1 ); | |||
There was a problem hiding this comment.
I'd say instead :
<?php
return (int)$rows[0][$orderField] + 1;There was a problem hiding this comment.
Ah, good point :)
| * Returns the attributes for this object, taken from the definition fields | ||
| * and function attributes. | ||
| * | ||
| * @return array |
There was a problem hiding this comment.
I'd add:
@see eZPersistentObject::definition()
|
Any news on this one? This is almost completely about PHPDocs, with just a few code changes concerning unused variables. As soon as this one is accepted, I will continue with the PHPDocs of another class :). |
… as done in #215
|
This last commit is the same as done in #215 - so if this pull request could be accepted, that one over there could be closed. |
|
Nice that you moved code changes out of this pull request, but there is quite a few other code changes left in here. |
|
Okay, I have reverted all code changes (except the preposition of Do you still see things that shouldn't be there? |
|
Looks good! |
|
Looks good to me too ! Merging :) |
PHPDocs for eZPersistentObject
To start a better support for our IDEs, I added PHPDocs to the eZPersistentObject class (and if this passes your acceptance tests, continue with other classes inheriting from it).
There are some very small modifications to the code, too, where I made sure that a return value has always the same type, but besides there should be no critical issues rising from these changes.
Originally, I wanted to make the class abstract (because classes inheriting from eZPersistetnObject have to implement the method
fetch(), but @lolautruche convinced me that this would be not the best idea.Best