TopEdits::getDisplayTitles() - Code Metrics - Inspection of "TopEdits: Show per-namespace breakdown if requeste..." - x-tools/xtools - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#104)

by MusikAnimal

created 2017-10-10 17:12 UTC

TopEdits::getDisplayTitles() A

↳ Parent: TopEdits

Complexity

Conditions	2
Paths	1

Size

Total Lines	14
Code Lines	8

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	14
rs	9.4285
c	0
b	0
f	0
cc	2
eloc	8
nc	1
nop	1

<?php
/**
 * This file contains only the TopEdits class.
 */

namespace Xtools;

use Symfony\Component\DependencyInjection\Container;
use DateTime;

/**
 * TopEdits returns the top-edited pages by a user. There is not a separate
 * repository because there is only one query :)
 */
class TopEdits extends Model
{
    /** @var Project The project. */
    protected $project;

    /** @var User The user. */
    protected $user;

    /** @var string[] Top edits object for quick caching, keyed by namespace ID. */
    protected $topEdits = [];

    /** @var int Number of rows to fetch. */
    protected $limit;

    /** @var int Which namespace we are querying for. */
    protected $namespace;

    const DEFAULT_LIMIT_SINGLE_NAMESPACE = 100;
    const DEFAULT_LIMIT_ALL_NAMESPACES = 20;

    /**
     * TopEdits constructor.
     * @param Project $project
     * @param User $user
     * @param string|int Namespace ID or 'all'.
     * @param int $limit Number of rows to fetch. This defaults to
     *   DEFAULT_LIMIT_SINGLE_NAMESPACE if $this->namespace is a single namespace (int),
     *   and DEFAULT_LIMIT_ALL_NAMESPACES if $this->namespace is 'all'.
     */
    public function __construct(Project $project, User $user, $namespace = 0, $limit = null)
    {
        $this->project = $project;
        $this->user = $user;
        $this->namespace = $namespace === 'all' ? 'all' : (int)$namespace;
class Id
{
    public $id;

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

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}

        if ($limit) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
            $this->limit = $limit;
        } else {
            $this->limit = $this->namespace === 'all'
                ? self::DEFAULT_LIMIT_ALL_NAMESPACES
                : self::DEFAULT_LIMIT_SINGLE_NAMESPACE;
        }
    }

    /**
     * Get the limit set on number of rows to fetch.
     * @return int
     */
    public function getLimit()
    {
        return $this->limit;
    }

    /**
     * Get the namespace set on the instance.
     * @return int|string Namespace ID or 'all'.
     */
    public function getNamespace()
    {
        return $this->namespace;
    }

    /**
     * Get the top edits by a user in the given namespace, or 'all' namespaces.
     * This is the public function that should be used.
     * @return string[] Results of self::getTopEditsByNamespace(), keyed by namespace.
     */
    public function getTopEdits()
    {
        if (count($this->topEdits) > 0) {
            return $this->topEdits;
        }

        /** @var string[] The top edited pages, keyed by namespace ID. */
        $topEditedPages = [];

        /** @var int[] Which namespaces to iterate over. */
        $namespaces = $this->namespace === 'all'

            ? array_keys($this->project->getNamespaces())
            : [$this->namespace];

        foreach ($namespaces as $nsId) {
            $pages = $this->getTopEditsByNamespace($nsId);

            if (count($pages)) {
                $topEditedPages[$nsId] = $pages;
            }
        }

        $this->topEdits = $topEditedPages;
        return $topEditedPages;
    }

    /**
     * Get the top edits by a user in the given namespace.
     * @param int $namespace Namespace ID.
     * @return string[] page_namespace, page_title, page_is_redirect,
     *   count (number of edits), assessment (page assessment).
     */
    protected function getTopEditsByNamespace($namespace = 0)
    {
        $topPages = $this->getRepository()->getTopEdits(
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
            $this->project,
            $this->user,
            $namespace,
            $this->limit
        );

        // Display titles need to be fetched ahead of time.
        $displayTitles = $this->getDisplayTitles($topPages);

        $pages = [];
        foreach ($topPages as $page) {
            // If non-mainspace, prepend namespace to the titles.
            $ns = (int) $page['page_namespace'];
            $nsTitle = $ns > 0 ? $this->project->getNamespaces()[$page['page_namespace']] . ':' : '';
            $pageTitle = $nsTitle . $page['page_title'];
            $page['displaytitle'] = $displayTitles[$pageTitle];
            // $page['page_title'] is retained without the namespace
            //  so we can link to TopEdits for that page.
            $page['page_title_ns'] = $pageTitle;
            $pages[] = $page;
        }

        return $pages;
    }

    /**
     * Get the display titles of the given pages.
     * @param  string[] $topPages As returned by $this->getRepository()->getTopEdits()
     * @return string[] Keys are the original supplied titles, and values are the display titles.
     */
    private function getDisplayTitles($topPages)
    {
        $namespaces = $this->project->getNamespaces();

        // First extract page titles including namespace.
        $pageTitles = array_map(function ($page) use ($namespaces) {
            // If non-mainspace, prepend namespace to the titles.
            $ns = $page['page_namespace'];
            $nsTitle = $ns > 0 ? $namespaces[$page['page_namespace']] . ':' : '';
            return $nsTitle . $page['page_title'];
        }, $topPages);

        return $this->getRepository()->getDisplayTitles($this->project, $pageTitles);
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
    }
}


1			<?php
2			/**
3			* This file contains only the TopEdits class.
4			*/
5
6			namespace Xtools;
7
8			use Symfony\Component\DependencyInjection\Container;
9			use DateTime;
10
11			/**
12			* TopEdits returns the top-edited pages by a user. There is not a separate
13			* repository because there is only one query :)
14			*/
15			class TopEdits extends Model
16			{
17			/** @var Project The project. */
18			protected $project;
19
20			/** @var User The user. */
21			protected $user;
22
23			/** @var string[] Top edits object for quick caching, keyed by namespace ID. */
24			protected $topEdits = [];
25
26			/** @var int Number of rows to fetch. */
27			protected $limit;
28
29			/** @var int Which namespace we are querying for. */
30			protected $namespace;
31
32			const DEFAULT_LIMIT_SINGLE_NAMESPACE = 100;
33			const DEFAULT_LIMIT_ALL_NAMESPACES = 20;
34
35			/**
36			* TopEdits constructor.
37			* @param Project $project
38			* @param User $user
39			* @param string\|int Namespace ID or 'all'.
40			* @param int $limit Number of rows to fetch. This defaults to
41			* DEFAULT_LIMIT_SINGLE_NAMESPACE if $this->namespace is a single namespace (int),
42			* and DEFAULT_LIMIT_ALL_NAMESPACES if $this->namespace is 'all'.
43			*/
44			public function __construct(Project $project, User $user, $namespace = 0, $limit = null)
45			{
46			$this->project = $project;
47			$this->user = $user;
48			$this->namespace = $namespace === 'all' ? 'all' : (int)$namespace;
			0 ignored issues – show Documentation Bug introduced 2017-10-10 17:15 UTC by Report Bug Copy Issue Report It seems like `$namespace === 'all' ? 'all' : (int) $namespace` can also be of type `string`. However, the property `$namespace` is declared as type `integer`. Maybe add an additional type check? Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly. For example, imagine you have a variable `$accountId` that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the `id` property of an instance of the `Account` class. This class holds a proper account, so the id value must no longer be false. Either this assignment is in error or a type check should be added for that assignment. class Id { public $id; public function __construct($id) { $this->id = $id; } } class Account { /** @var Id $id */ public $id; } $account_id = false; if (starsAreRight()) { $account_id = new Id(42); } $account = new Account(); if ($account instanceof Id) { $account->id = $account_id; } Loading history... Unused Code Bug introduced 2017-10-10 16:55 UTC by Report Bug Copy Issue Report The strict comparison `===` seems to always evaluate to `false` as the types of `$namespace` (`integer`) and `'all'` (`string`) can never be identical. Maybe you want to use a loose comparison `==` instead? Loading history...
49
50			if ($limit) {
			0 ignored issues – show Bug Best Practice introduced 2017-10-10 16:55 UTC by Report Bug Copy Issue Report The expression `$limit` of type `integer\|null` is loosely compared to `true`; this is ambiguous if the integer can be zero. You might want to explicitly use `!== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `integer` values, zero is a special case, in particular the following results might be unexpected: 0 == false // true 0 == null // true 123 == false // false 123 == null // false // It is often better to use strict comparison 0 === false // false 0 === null // false Loading history...
51			$this->limit = $limit;
52			} else {
53			$this->limit = $this->namespace === 'all'
54			? self::DEFAULT_LIMIT_ALL_NAMESPACES
55			: self::DEFAULT_LIMIT_SINGLE_NAMESPACE;
56			}
57			}
58
59			/**
60			* Get the limit set on number of rows to fetch.
61			* @return int
62			*/
63			public function getLimit()
64			{
65			return $this->limit;
66			}
67
68			/**
69			* Get the namespace set on the instance.
70			* @return int\|string Namespace ID or 'all'.
71			*/
72			public function getNamespace()
73			{
74			return $this->namespace;
75			}
76
77			/**
78			* Get the top edits by a user in the given namespace, or 'all' namespaces.
79			* This is the public function that should be used.
80			* @return string[] Results of self::getTopEditsByNamespace(), keyed by namespace.
81			*/
82			public function getTopEdits()
83			{
84			if (count($this->topEdits) > 0) {
85			return $this->topEdits;
86			}
87
88			/** @var string[] The top edited pages, keyed by namespace ID. */
89			$topEditedPages = [];
90
91			/** @var int[] Which namespaces to iterate over. */
92			$namespaces = $this->namespace === 'all'
			0 ignored issues – show Unused Code Bug introduced 2017-10-10 17:15 UTC by Report Bug Copy Issue Report The strict comparison `===` seems to always evaluate to `false` as the types of `$this->namespace` (`integer`) and `'all'` (`string`) can never be identical. Maybe you want to use a loose comparison `==` instead? Loading history...
93			? array_keys($this->project->getNamespaces())
94			: [$this->namespace];
95
96			foreach ($namespaces as $nsId) {
97			$pages = $this->getTopEditsByNamespace($nsId);
98
99			if (count($pages)) {
100			$topEditedPages[$nsId] = $pages;
101			}
102			}
103
104			$this->topEdits = $topEditedPages;
105			return $topEditedPages;
106			}
107
108			/**
109			* Get the top edits by a user in the given namespace.
110			* @param int $namespace Namespace ID.
111			* @return string[] page_namespace, page_title, page_is_redirect,
112			* count (number of edits), assessment (page assessment).
113			*/
114			protected function getTopEditsByNamespace($namespace = 0)
115			{
116			$topPages = $this->getRepository()->getTopEdits(
			1 ignored issue – show Bug introduced 2017-10-10 16:55 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Xtools\Repository` as the method `getTopEdits()` does only exist in the following sub-classes of `Xtools\Repository`: `Xtools\TopEditsRepository`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
117			$this->project,
118			$this->user,
119			$namespace,
120			$this->limit
121			);
122
123			// Display titles need to be fetched ahead of time.
124			$displayTitles = $this->getDisplayTitles($topPages);
125
126			$pages = [];
127			foreach ($topPages as $page) {
128			// If non-mainspace, prepend namespace to the titles.
129			$ns = (int) $page['page_namespace'];
130			$nsTitle = $ns > 0 ? $this->project->getNamespaces()[$page['page_namespace']] . ':' : '';
131			$pageTitle = $nsTitle . $page['page_title'];
132			$page['displaytitle'] = $displayTitles[$pageTitle];
133			// $page['page_title'] is retained without the namespace
134			// so we can link to TopEdits for that page.
135			$page['page_title_ns'] = $pageTitle;
136			$pages[] = $page;
137			}
138
139			return $pages;
140			}
141
142			/**
143			* Get the display titles of the given pages.
144			* @param string[] $topPages As returned by $this->getRepository()->getTopEdits()
145			* @return string[] Keys are the original supplied titles, and values are the display titles.
146			*/
147			private function getDisplayTitles($topPages)
148			{
149			$namespaces = $this->project->getNamespaces();
150
151			// First extract page titles including namespace.
152			$pageTitles = array_map(function ($page) use ($namespaces) {
153			// If non-mainspace, prepend namespace to the titles.
154			$ns = $page['page_namespace'];
155			$nsTitle = $ns > 0 ? $namespaces[$page['page_namespace']] . ':' : '';
156			return $nsTitle . $page['page_title'];
157			}, $topPages);
158
159			return $this->getRepository()->getDisplayTitles($this->project, $pageTitles);
			1 ignored issue – show Bug introduced 2017-10-10 16:55 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Xtools\Repository` as the method `getDisplayTitles()` does only exist in the following sub-classes of `Xtools\Repository`: `Xtools\TopEditsRepository`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
160			}
161			}
162

x-tools / xtools

Pull Request — master (#104)

TopEdits::getDisplayTitles() A

Complexity

Size

Duplication

Importance

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like