| ezcFeed |
| ======= |
| |
| - Why is there a module "name" and a "prefix"? Doesn't one of them last? Would |
| make it unembigious. |
| |
| # Some modules have the same name as RSS/ATOM elements (eg. 'content' module and |
| ATOM 'content' property. If we want to access properties and modules the same, |
| it will be ambiguous ($feed->content refering to ATOM property or content |
| module). So the content module has the 'Content' name and 'content' prefix to |
| differentiate this. |
| |
| - ezcFeed::addModule() -> todo. |
| |
| # Removed. |
| |
| - How to add a custom module to ezcFeed? |
| * A new module can be defined by creating a class which extends the class |
| * {@link ezcFeedModule}, and adding it to the {@link self::$supportedModules} |
| * and {@link self::$supportedModulesPrefixes} arrays. |
| Is it necessary to extend ezcFeed to do this? |
| |
| # I did not find a better way to do this (I don't know how to see at runtime which |
| modules are defined without hard-coding it in ezcFeed). |
| |
| # Use static methods like ezcFeed::registerModule(), |
| ezcFeed::unregisterModule(), which add or remove a module. |
| |
| # Added the functions registerFeed, registerModule and unregisterFeed, |
| unregisterModule to ezcFeed. |
| |
| - Why does ezcFeed have double single/multiple value properties ( |
| $author[s],...). |
| |
| # I think we should only go for the plural property and if a feed type |
| # supports only 1 element of that type, use the first in the array. |
| |
| I changed it so that only the singular is needed (eg. $feed->item will |
| return an array of ezcFeedItem objects). |
| |
| - ezcFeed::__get() makes use of isset( $this->$property ), this also returns |
| true for real protected/private properties. It should call $this->__isset() |
| directly. This is also more efficient. |
| |
| # Fixed. |
| |
| - ezcFeed::__get() should also better throw an ezcBasePropertyNotFoundException. |
| |
| # In this case, when parsing a feed you will get an exception if trying to fetch |
| $feed->title and the title is missing from the XML document. |
| |
| # This exception should be issued in the parser then, not in __get(). |
| |
| # I don't think it is a good idea to throw an exception if the XML which |
| is parsed is missing a title or any element. All XML feeds should be |
| parseable. |
| |
| - ezcFeed::__isset() behaviour incorrect. We return true if a property exists |
| but is null. |
| |
| # Fixed. |
| |
| - Why does ezcFeed declare protected attributes and mark them as @ignore? No |
| class seems to extend ezcFeed. Declaring this private makes more sense. |
| |
| # Maybe somebody will want to extend ezcFeed so it would be easier with |
| protected instead of private attributes (see also issue tracker for requests |
| to change private attributes into protected ones). |
| |
| # Then we must declar it like this. If we intend people to extend our |
| classes, we mark the properties they might need "protected". If this is |
| not the case, we keep them private. That is the sense of these keywords. |
| But beware, if we mark the protected, they must be API stable and may not |
| change later. Protected and @ignore (even better @access private) should |
| only be used, if we extend the classes internally in the component, but do |
| not want to expose the affected elements as stable API to others. |
| |
| # I changed some methods and variables from protected to private. |
| |
| - Huge docblock at the beginning of the file is annoying. We should think |
| about a way to include docs there. |
| |
| # I prefer to write longer docblocks to help a bit the people who want to jump |
| in without reading the tutorial first or asking on IRC. |
| |
| # We should think about outsourcing such huge examples and use the @example |
| tag for them. This tag receives an external source file name and puts this |
| example in the generated docs. We keep it in the online docs this way and |
| make source code browsing more comfortable. |
| |
| - Why ezcFeedCanNotParseException and ezcFeedParseErrorException? The name |
| states the same purpose. Even the docs are the same. |
| |
| # I removed ezcFeedCanNotParseException and used the other one only. |
| |
| ezcFeedItem |
| =========== |
| |
| - Why are similar feed item elements handled so different? Example: |
| - For atom links you have $link->href, $link->rel |
| - Form rss links you have $link->get() |
| I would prefer to access them in an equal manor. For example, the link |
| itself could always be accessed via $link->href and only for atom the other |
| properties could be available. |
| |
| # I changed all feed types and modules to be used in a more uniform manner. |
| All feed and module elements now have a type (struct) (eg: link, category, |
| person) and the data is stored in these structs. When generating or parsing |
| a feed, the data is fetched from these structs or the structs are filled with |
| data from XML, respectively. The unused data is ignored. |
| |
| Example: |
| RSS2 XML: <link>http://ez.no/</link> |
| ATOM XML: <link href="http://ez.no/" title="eZ Systems" type="text/html" /> |
| |
| Parsed struct: |
| RSS2: ezcFeedLinkElement{ 'href' => 'http://ez.no/', |
| 'title' => null, |
| 'type' => null } |
| |
| ATOM: ezcFeedLinkElement{ 'href' => 'http://ez.no/', |
| 'title' => 'eZ Systems', |
| 'type' => 'text/html' } |
| |
| parsing a feed: |
| $feed = ezcFeed::parse( /* feed url */ ); |
| foreach ( $feed->link as $link ) |
| { |
| $href = $link->href; |
| $title = $link->title; |
| $type = $link->type; |
| } |
| |
| creating a feed: |
| $feed = new ezcFeed(); // the type of feed is not required anymore |
| $link = $feed->add( 'link' ); |
| $link->href = 'http://ez.no/'; |
| $link->title = 'eZ Systems'; |
| $link->type = 'text/html'; |
| |
| $xml = $feed->generate( 'rss2' ); // the feed type is specified when calling generate() |
| // results: <link>http://ez.no/</link> |
| |
| $xml = $feed->generate( 'atom' ); |
| // results: <link href="http://ez.no/" title="eZ Systems" type="text/html" /> |
| |
| - Handling of the $id attribute is not working as noted in the docs. :: |
| |
| $item->id = 'http://example.com/someentry'; |
| $item->id->isPermalink = true; |
| |
| Gives: :: |
| |
| Fatal error: Cannot use object of type ezcFeedElement as array in |
| /usr/share/ezcomponents/trunk/Feed/src/processors/rss2.php on line 509 |
| |
| Removing the array accesses there fixes the fatal error, however, the |
| isPermalink attribute is not generated anyway and the result is a warining: :: |
| |
| Warning: DOMDocument::createElement(): unterminated entity reference |
| Photos in /usr/share/ezcomponents/trunk/Feed/src/interfaces/processor.php on |
| line 122 |
| |
| # This should be fixed now. |
| |
| The parse handling of this one looks rather scary in the docs. |
| |
| # How is it scary? And where exactly? |
| |
| ezcFeedProcessor |
| ================ |
| |
| - Why does ezcFeedProcessor use get()/set() instead of |
| __set()/__get()/__isset()? |
| |
| # Changed get() and set() into __get() and __set(), and added __isset() to |
| ezcFeedProcessor. |
| |
| ezcFeedRss2 |
| =========== |
| |
| - The protected static attribute $rss2Schema contains a huge (122 lines) |
| definition for a 3 dimensional array. It looks rather unmaintainable . The |
| documentation "Holds the RSS2 feed schema." does not tell anything about its |
| purpose. |
| |
| # I removed the schemas. |
| |
| - The generateItems() method is 130 lines long, pure code and not a single |
| line of inline docs, and looks absolutly unmaintainable and untestable. |
| |
| # Sometimes the methods cannot be smaller. How is it unmaintainable and |
| untestable? There is only an foreach and a switch. |
| |
| - I suspect the other parser/processor classes look similar. |
| |
| # They look similar. Not much to do to make them smaller. |
| |
| ezcFeedTools |
| ============ |
| |
| - Should be private. No sense in making such utility classes public if they |
| are not of special interesst for foreigners. |
| |
| # prepareDate() could be useful for some people. But the other functions are |
| not so important. I will consider making it private. I wanted to add some |
| other functions to this class, related to date formatting for different |
| feed types. |
| |
| # See discussion on the ML. |
| |
| - ezcFeedTools::getAttributes() is a duplication of |
| DOMElement::getAttribute(). |
| |
| # I must have missed that. I will deprecate the function. |
| |
| - ezcFeedTools::getAttributes() looks pretty much unnesseccary. |
| DOMNode->$attributes can be used instead for iteration, |
| DOMElement->getAttribute() for access by name. |
| |
| # I must have missed that. I will deprecate the function. |
| |
| - ezcFeedTools::prepareDate() should not accept DateTime objects. If it |
| receives them, this indicates bad code. |
| |
| # What if you assign a DateTime object to $feed->published for example? Should |
| the code throw an exception because the date was not a string or timestamp? |
| |
| # The exception for assigning DateTime objects should be made in __set(). |
| This way it is clear, that this is the intended behaviour and that other |
| data assigned must be parsed. See discussion on the ML. |
| |
| - ezcFeedTools::normalizeName() should use isset(). Works here and is faster. |
| |
| # I will update this. |
| |
| - The Feed component depends on the XML prefix that is used, but should |
| instead rely on the full XML name space instead. |
| |
| # Done. The getModuleName(), getNamespace() and getNamespacePrefix() functions |
| from ezcFeedModule and children were changed to static to allow this (and |
| it is more logical to have them static). |
| |
| General |
| ======= |
| |
| - A lot of multi-line short descriptions for methods, which do not parse |
| properly. The very first line of the doc bloc is the short description, all |
| following belong to the long description. |
| |
| # It seems that phpdoc handles this well now. |
| |
| - The test suite consist only of regression tests and 1 single unit test. |
| Removing the regression tests the overall code coverage is 37.57, which is |
| completly unacceptable. Especially since there are 166 public/protected |
| methods to ensure BC for. |
| |
| # Why would you remove the regression tests? They don't consume so much memory |
| as the template regression tests, so there is no reason to remove them. |
| |
| - Test cases are time zone dependent :: |
| |
| # In 5.3 no tests fail, but in 5.2.* six tests fail because of timezone. I don't |
| know how to fix them yet. |
| |
| 1) /home/dotxp/dev/PHP/actual/ezcomponents/trunk/Feed/tests/atom/regression/generate/modules/dc/dc_all_lang_multiple.in(ezcFeedAtomRegressionGenerateTest) |
| The dc_all_lang_multiple.out is not the same as the generated feed from dc_all_lang_multiple.in. |
| Failed asserting that two strings are equal. |
| --- Expected |
| +++ Actual |
| @@ -18,8 +18,8 @@ |
| <dc:coverage xml:lang="d">DC coverage 2</dc:coverage> |
| <dc:creator xml:lang="e">DC creator 1</dc:creator> |
| <dc:creator xml:lang="f">DC creator 2</dc:creator> |
| - <dc:date xml:lang="g">2008-02-14T13:34:51+00:00</dc:date> |
| - <dc:date xml:lang="h">1970-01-01T00:00:00+00:00</dc:date> |
| + <dc:date xml:lang="g">2008-02-14T13:34:51+01:00</dc:date> |
| + <dc:date xml:lang="h">1970-01-01T00:00:00+01:00</dc:date> |
| <dc:description xml:lang="i">DC description 1</dc:description> |
| <dc:description xml:lang="j">DC description 2</dc:description> |
| <dc:format xml:lang="k">DC format 1</dc:format> |
| |
| /home/dotxp/dev/PHP/actual/ezcomponents/trunk/Feed/tests/atom/atom_regression_generate_test.php:65 |
| /home/dotxp/dev/PHP/actual/ezcomponents/trunk/Feed/tests/regression_test.php:269 |
| |
| + 5 other similar ones |
| |