NewbbOnlineHandler - Code Metrics - Inspection of "Merge pull request #52 from geekwright/issue50" - mambax7/newbb5 - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( e4a341...7497e8 )

by Michael

created 2017-09-26 16:08 UTC

NewbbOnlineHandler C

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	351
Duplicated Lines	17.38 %

Coupling/Cohesion

Components	1
Dependencies	0

Importance

Changes

Metric	Value
dl	61
loc	351
rs	6.8
c	0
b	0
f	0
wmc	55
lcom	1
cbo	0

10 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	4	1
A	init()	15	20	4
B	update()	0	28	6
C	render()	23	55	10
C	showOnline()	23	57	10
B	write()	0	49	6
A	gc()	0	9	1
B	getAll()	0	25	6
C	checkStatus()	0	28	7
A	getCount()	0	13	4

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like NewbbOnlineHandler often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use NewbbOnlineHandler, and based on these observations, apply Extract Interface, too.

<?php

/**
 * NewBB 5.0x,  the forum module for XOOPS project
 *
 * @copyright      XOOPS Project (https://xoops.org)
 * @license        GNU GPL 2 or later (http://www.gnu.org/licenses/gpl-2.0.html)
 * @author         Taiwen Jiang (phppp or D.J.) <[email protected]>
 * @since          4.00
 * @package        module::newbb
 */

use Xmf\IPAddress;

// defined('XOOPS_ROOT_PATH') || exit('Restricted access.');


include_once __DIR__ . '/../include/functions.config.php';

/**
 * Class NewbbOnlineHandler
 */
class NewbbOnlineHandler
namespace YourVendor;

class YourClass { }
{
    public $db;
    public $forum_id;
    public $forumObject;
    public $topic_id;
    public $user_ids = [];

    public function __construct(XoopsDatabase $db)
    {
        $this->db = $db;
    }

    /**
     * @param null|\NewbbForum $forum
     * @param null|Topic       $forumtopic
     */
    public function init($forum = null, $forumtopic = null)
    {
        if (is_object($forum)) {

            $this->forum_id     = $forum->getVar('forum_id');
            $this->forumObject = $forum;
        } else {
            $this->forum_id     = (int)$forum;
            $this->forumObject = $forum;
        }
        if (is_object($forumtopic)) {

            $this->topic_id = $forumtopic->getVar('topic_id');
            if (empty($this->forum_id)) {
                $this->forum_id = $forumtopic->getVar('forum_id');
            }
        } else {
            $this->topic_id = (int)$forumtopic;
        }

        $this->update();
    }

    public function update()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        global $xoopsModule;
function myFunction($a, $b) {
    // Do something
}

        mt_srand((double)microtime() * 1000000);
        // set gc probabillity to 10% for now..
        if (mt_rand(1, 100) < 60) {
            $this->gc(150);
        }
        if (is_object($GLOBALS['xoopsUser'])) {
            $uid   = $GLOBALS['xoopsUser']->getVar('uid');
            $uname = $GLOBALS['xoopsUser']->getVar('uname');
            $name  = $GLOBALS['xoopsUser']->getVar('name');
        } else {
            $uid   = 0;
            $uname = '';
            $name  = '';
        }

        $xoops_onlineHandler = xoops_getHandler('online');
        $xoopsupdate         = $xoops_onlineHandler->write($uid, $uname, time(), $xoopsModule->getVar('mid'), Xmf\IPAddress::fromRequest()->asReadable());
        if (!$xoopsupdate) {
if (rand(1, 6) > 3) {
//print "Check failed";
} else {
    print "Check succeeded";
}
            //xoops_error("newbb online upate error");
        }

        $uname = (empty($GLOBALS['xoopsModuleConfig']['show_realname']) || empty($name)) ? $uname : $name;
        $this->write($uid, $uname, time(), $this->forum_id, IPAddress::fromRequest()->asReadable(), $this->topic_id);
    }

    /**
     * @param $xoopsTpl
     */
    public function render(Smarty $xoopsTpl)
    {
        include_once __DIR__ . '/../include/functions.render.php';
        include_once __DIR__ . '/../include/functions.user.php';
        $criteria = null;
        if ($this->topic_id) {

            $criteria = new Criteria('online_topic', $this->topic_id);
        } elseif ($this->forum_id) {
            $criteria = new Criteria('online_forum', $this->forum_id);
        }
        $users     = $this->getAll($criteria);
        $num_total = count($users);

        $num_user     = 0;
        $users_id     = [];
        $users_online = [];
        for ($i = 0; $i < $num_total; ++$i) {

            if (empty($users[$i]['online_uid'])) {
                continue;
            }
            $users_id[]                             = $users[$i]['online_uid'];
            $users_online[$users[$i]['online_uid']] = [
                'link'  => XOOPS_URL . '/userinfo.php?uid=' . $users[$i]['online_uid'],
                'uname' => $users[$i]['online_uname']
            ];
            ++$num_user;
        }
        $num_anonymous           = $num_total - $num_user;
        $online                  = [];
        $online['image']         = newbbDisplayImage('whosonline');
        $online['num_total']     = $num_total;
        $online['num_user']      = $num_user;
        $online['num_anonymous'] = $num_anonymous;
        $administrator_list      = newbbIsModuleAdministrators($users_id);
        $moderator_list          = [];
        if ($member_list = array_diff(array_keys($administrator_list), $users_id)) {

            if (is_object($this->forumObject)) {
                $moderator_list = $this->forumObject->getVar('forum_moderator');
            } else {
                $moderator_list = newbbIsForumModerators($member_list);
            }
        }
        foreach ($users_online as $uid => $user) {
            if (!empty($administrator_list[$uid])) {
                $user['level'] = 2;
            } elseif (!empty($moderator_list[$uid])) {
                $user['level'] = 1;
            } else {
                $user['level'] = 0;
            }
            $online['users'][] = $user;
        }

        $xoopsTpl->assign_by_ref('online', $online);
    }

    /**
     * Deprecated
     */
    public function showOnline()
    {
        include_once __DIR__ . '/../include/functions.render.php';
        include_once __DIR__ . '/../include/functions.user.php';
        $criteria = null;
        if ($this->topic_id) {

            $criteria = new Criteria('online_topic', $this->topic_id);
        } elseif ($this->forum_id) {
            $criteria = new Criteria('online_forum', $this->forum_id);
        }
        $users     = $this->getAll($criteria);
        $num_total = count($users);

        $num_user     = 0;
        $users_id     = [];
        $users_online = [];
        for ($i = 0; $i < $num_total; ++$i) {

            if (empty($users[$i]['online_uid'])) {
                continue;
            }
            $users_id[]                             = $users[$i]['online_uid'];
            $users_online[$users[$i]['online_uid']] = [
                'link'  => XOOPS_URL . '/userinfo.php?uid=' . $users[$i]['online_uid'],
                'uname' => $users[$i]['online_uname']
            ];
            ++$num_user;
        }
        $num_anonymous           = $num_total - $num_user;
        $online                  = [];
        $online['image']         = newbbDisplayImage('whosonline');
        $online['statistik']     = newbbDisplayImage('statistik');
        $online['num_total']     = $num_total;
        $online['num_user']      = $num_user;
        $online['num_anonymous'] = $num_anonymous;
        $administrator_list      = newbbIsModuleAdministrators($users_id);
        $moderator_list          = [];
        if ($member_list = array_diff($users_id, array_keys($administrator_list))) {

            if (is_object($this->forumObject)) {
                $moderator_list = $this->forumObject->getVar('forum_moderator');
            } else {
                $moderator_list = newbbIsForumModerators($member_list);
            }
        }

        foreach ($users_online as $uid => $user) {
            if (in_array($uid, $administrator_list)) {
                $user['level'] = 2;
            } elseif (in_array($uid, $moderator_list)) {
                $user['level'] = 1;
            } else {
                $user['level'] = 0;
            }
            $online['users'][] = $user;
        }

        return $online;
    }

    /**
     * Write online information to the database
     *
     * @param  int    $uid      UID of the active user
     * @param  string $uname    Username
     * @param         $time
     * @param  string $forum_id Current forum_id
     * @param  string $ip       User's IP adress
     * @param         $topic_id
     * @return bool   TRUE on success
     * @internal param string $timestamp
     */
    public function write($uid, $uname, $time, $forum_id, $ip, $topic_id)
    {
        global $xoopsModule, $xoopsDB;
function myFunction($a, $b) {
    // Do something
}

        $uid = (int)$uid;
        if ($uid > 0) {
            $sql = 'SELECT COUNT(*) FROM ' . $this->db->prefix('newbb_online') . ' WHERE online_uid=' . $uid;
        } else {
            $sql = 'SELECT COUNT(*) FROM ' . $this->db->prefix('newbb_online') . ' WHERE online_uid=' . $uid . " AND online_ip='" . $ip . "'";
        }
        list($count) = $this->db->fetchRow($this->db->queryF($sql));
        if ($count > 0) {
            $sql = 'UPDATE ' . $this->db->prefix('newbb_online') . " SET online_updated= '" . $time . "', online_forum = '" . $forum_id . "', online_topic = '" . $topic_id . "' WHERE online_uid = " . $uid;
            if (0 == $uid) {
                $sql .= " AND online_ip='" . $ip . "'";
            }
        } else {
            $sql = sprintf('INSERT INTO %s (online_uid, online_uname, online_updated, online_ip, online_forum, online_topic) VALUES (%u, %s, %u, %s, %u, %u)', $this->db->prefix('newbb_online'), $uid, $this->db->quote($uname), $time, $this->db->quote($ip), $forum_id, $topic_id);
        }
        if (!$this->db->queryF($sql)) {
            //xoops_error($this->db->error());

            return false;
        }

        /** @var XoopsOnlineHandler $xoops_onlineHandler */
        $xoops_onlineHandler = xoops_getHandler('online');
        $xoopsOnlineTable    = $xoops_onlineHandler->table;

        $sql = 'DELETE FROM '
               . $this->db->prefix('newbb_online')
               . ' WHERE'
               . ' ( online_uid > 0 AND online_uid NOT IN ( SELECT online_uid FROM '
               . $xoopsOnlineTable
               . ' WHERE online_module ='
               . $xoopsModule->getVar('mid')
               . ' ) )'
               . ' OR ( online_uid = 0 AND online_ip NOT IN ( SELECT online_ip FROM '
               . $xoopsOnlineTable
               . ' WHERE online_module ='
               . $xoopsModule->getVar('mid')
               . ' AND online_uid = 0 ) )';

        if ($result = $this->db->queryF($sql)) {
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
            return true;
        } else {
            //xoops_error($this->db->error());

            return false;
        }
    }

    /**
     * Garbage Collection
     *
     * Delete all online information that has not been updated for a certain time
     *
     * @param int $expire Expiration time in seconds
     */
    public function gc($expire)
    {
        global $xoopsModule;
function myFunction($a, $b) {
    // Do something
}
        $sql = 'DELETE FROM ' . $this->db->prefix('newbb_online') . ' WHERE online_updated < ' . (time() - (int)$expire);
        $this->db->queryF($sql);

        $xoops_onlineHandler = xoops_getHandler('online');
        $xoops_onlineHandler->gc($expire);
    }

    /**
     * Get an array of online information
     *
     * @param  CriteriaElement $criteria {@link CriteriaElement}

     * @return array           Array of associative arrays of online information
     */
    public function getAll(CriteriaElement $criteria = null)
    {
        $ret   = [];
        $limit = $start = 0;
        $sql   = 'SELECT * FROM ' . $this->db->prefix('newbb_online');
        if (is_object($criteria) && is_subclass_of($criteria, 'criteriaelement')) {

            $sql   .= ' ' . $criteria->renderWhere();
            $limit = $criteria->getLimit();
            $start = $criteria->getStart();
        }
        $result = $this->db->query($sql, $limit, $start);
        if (!$result) {
            return $ret;
        }
        while ($myrow = $this->db->fetchArray($result)) {
            $ret[] = $myrow;
            if ($myrow['online_uid'] > 0) {
                $this->user_ids[] = $myrow['online_uid'];
            }
            unset($myrow);
        }
        $this->user_ids = array_unique($this->user_ids);

        return $ret;
    }

    /**
     * @param $uids
     * @return array
     */
    public function checkStatus($uids)
    {
        $online_users = [];
        $ret          = [];
        if (!empty($this->user_ids)) {
            $online_users = $this->user_ids;
        } else {
            $sql = 'SELECT online_uid FROM ' . $this->db->prefix('newbb_online');
            if (!empty($uids)) {
                $sql .= ' WHERE online_uid IN (' . implode(', ', array_map('intval', $uids)) . ')';
            }

            $result = $this->db->query($sql);
            if (!$result) {
                return $ret;
            }
            while (list($uid) = $this->db->fetchRow($result)) {
                $online_users[] = $uid;
            }
        }
        foreach ($uids as $uid) {
            if (in_array($uid, $online_users)) {
                $ret[$uid] = 1;
            }
        }

        return $ret;
    }

    /**
     * Count the number of online users
     *
     * @param  CriteriaElement $criteria {@link CriteriaElement}

     * @return bool
     */
    public function getCount(CriteriaElement $criteria = null)
    {
        $sql = 'SELECT COUNT(*) FROM ' . $this->db->prefix('newbb_online');
        if (is_object($criteria) && is_subclass_of($criteria, 'criteriaelement')) {

            $sql .= ' ' . $criteria->renderWhere();
        }
        if (!$result = $this->db->query($sql)) {
            return false;
        }
        list($ret) = $this->db->fetchRow($result);

        return $ret;
    }
}


Push — master ( e4a341...7497e8 )

NewbbOnlineHandler C

Complexity

Size/Duplication

Coupling/Cohesion

Importance

10 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

mambax7 / newbb5

Push — master ( e4a341...7497e8 )

NewbbOnlineHandler C

Complexity

Size/Duplication

Coupling/Cohesion

Importance

10 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like