OUGC Pages Review

[ZiNgA BuRgA]

This "review" was requested by Sama34.

Rather late with this one, but I managed to get around it.  I haven't been doing PHP coding much lately, so my memory is a bit weak, meaning that I've likely glossed over much more than I usually would do.

Anyway, the plugin generally seems to be fine.  Some suggestions I'd make:

• It's probably not clear what one needs to exactly do to integrate Pages into the forum.  By default, no links are added.

• PHP Code:

  $is_page = $mybb->get_input('page') && !empty($mybb->cache->cache['ougc_pages']['pages'][$mybb->get_input('page')]);

  ---

  Variable not used anywhere, consider removing.

• PHP Code:

  $query = $db->simple_select('ougc_pages', 'pid, url', 'visible=\'1\''.(!empty($update['categories']) ? ' AND cid NOT IN (\''.implode('\', \'', $update['categories']).'\')' : ''), array('order_by' => 'disporder'));

  ---

  The attempt to filter out categories doesn't make a lot of sense, since the rest of the code seems to assume that all pages are stored in the cache.
  The filter doesn't do anything anyway - I assume that the implode part was supposed to be
  

  PHP Code:

  implode('\', \'', array_keys($update['categories']))

  ---

  (though this obviously breaks things)
  My guess is that it's legacy code that forgot to be removed.

• The categories cache (MyBB level cache, not the cache that is tied to the class) doesn't appear to be used.  Perhaps if it was keyed by URL, like the pages cache, it could be more useful.
• My mistake with this one:
  
  PHP Code:

  // convert \xA0 to spaces (reverse  ) $message = trim(preg_replace(array('~ {2,}~', "~\n{2,}~"), array(' ', "\n"), strtr($message, array("\xA0" => ' ', "\r" => '', "\t" => ' '))));

  ---

  "\xA0" should be utf8_encode("\xA0")
  (though I can't actually see this code being used anywhere in the plugin - my guess is that it's just a generic include across all OUGC plugins?)

• I personally don't agree with stuffing all the code into the ougc_pages.php file.  For example, the ougc_pages_show function is only ever called from pages.php, so it may as well be placed there so that it doesn't need to be loaded on every page load.  I suppose this is more of a design decision though.