[FeedExpander] Deal with empty item #535
Conversation
There was a problem hiding this comment.
Nice addition, I like it quite a lot 👍
This PR has two topics mixed:
- The changes to the core API (FeedExpander)
- Adding a new bridge (FilterBridge)
Please split these changes into separate PRs. Since FilterBridge depends on the changes to FeedExpander, we need to merge the latter one first, so I based my review on the changes to FeedExpander.
lib/FeedExpander.php
Outdated
| @@ -48,7 +48,9 @@ protected function collect_RSS_1_0_data($rssContent, $maxItems){ | |||
| $this->load_RSS_2_0_feed_data($rssContent->channel[0]); | |||
| foreach($rssContent->item as $item){ | |||
| debugMessage('parsing item ' . var_export($item, true)); | |||
| $this->items[] = $this->parseItem($item); | |||
| if (null !== $this->parseItem($item)) { | |||
There was a problem hiding this comment.
$this->parseItem($item) will return an empty array for native types (see parseRSS_1_0_Item for example), so instead of checking for null check if the array is empty: if(empty($this->parseItem($item)){
lib/FeedExpander.php
Outdated
| @@ -48,7 +48,9 @@ protected function collect_RSS_1_0_data($rssContent, $maxItems){ | |||
| $this->load_RSS_2_0_feed_data($rssContent->channel[0]); | |||
| foreach($rssContent->item as $item){ | |||
| debugMessage('parsing item ' . var_export($item, true)); | |||
| $this->items[] = $this->parseItem($item); | |||
| if (null !== $this->parseItem($item)) { | |||
| $this->items[] = $this->parseItem($item); | |||
There was a problem hiding this comment.
This is calling the same function as in the if-statement. Since parseItem could potentially load additional data during execution its result should be put into a variable instead of calling it twice.
Frenzie
changed the title
See the comment RSS-Bridge#402 (comment) Split off from RSS-Bridge#535
|
Thanks for updating 👍 |
See the comment RSS-Bridge#402 (comment) Split off from RSS-Bridge#535
[FeedExpander] Deal with empty item
See the comment #402 (comment)