PlayerController::listAction() - Code Metrics - Inspection of "Move player sorting to SQL instead of PHP" - allejo/bzion - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — feature/player-list ( ee00f1...3478af )

by Vladimir

created 2017-07-21 02:42 UTC

PlayerController::listAction() D

↳ Parent: PlayerController

Complexity

Conditions	17
Paths	200

Size

Total Lines	75
Code Lines	43

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	0
CRAP Score	306

Importance

Changes

Metric	Value
dl	0
loc	75
ccs	0
cts	41
cp	0
rs	4.9162
c	0
b	0
f	0
cc	17
eloc	43
nc	200
nop	3
crap	306

How to fix Long Method Complexity

<?php

use BZIon\Form\Creator\PlayerAdminNotesFormCreator as FormCreator;
use Symfony\Component\Form\Form;
use Symfony\Component\HttpFoundation\Request;

class PlayerController extends JSONController
{
    private $creator;

    public function showAction(Player $player, Player $me, Request $request)
    {
        if ($me->hasPermission(Permission::VIEW_VISITOR_LOG)) {
            $this->creator = new FormCreator($player);
            $form = $this->creator->create()->handleRequest($request);

            if ($form->isValid()) {
                $form = $this->handleAdminNotesForm($form, $player, $me);

            }

            $formView = $form->createView();
        } else {
            // Don't spend time rendering the form unless we need it
            $formView = null;
        }

        return array(
            "player"         => $player,
            "adminNotesForm" => $formView,
        );
    }

    public function editAction(Player $player, Player $me)
    {
        if (!$me->canEdit($player)) {
            throw new ForbiddenException("You are not allowed to edit other players");
        }

        $params = array(
            'me'   => $player,
            'self' => false,
        );

        return $this->forward('edit', $params, 'Profile');
    }

    public function listAction(Request $request, Player $me, Team $team = null)
    {
        $query = Player::getQueryBuilder();

        // Load all countries into the cache so they are ready for later
        Country::getQueryBuilder()->addToCache();

        if ($team) {
            $query->where('team')->is($team);
        } else {
            // Add all teams to the cache
            $this->getQueryBuilder('Team')
                ->where('members')->greaterThan(0)
                ->addToCache();
        }

        if ($request->query->has('exceptMe')) {
            $query->except($me);
        }

        $groupBy = $request->query->get('groupBy');
        $sortBy = $request->query->get('sortBy');
        $sortOrder = $request->query->get('sortOrder');

        $query->withMatchActivity();

        if ($sortBy || $sortOrder) {
            $sortBy = $sortBy ? $sortBy : 'callsign';
            $sortOrder = $sortOrder ? $sortOrder : 'ASC';

            if ($sortBy == 'activity') {
                $query->sortBy('activity');
            }
            elseif ($sortBy == 'callsign') {
                $query->sortBy('name');
            }

            if ($sortOrder == 'DESC') {
                $query->reverse();
            }
        }

        $players = $query->getModels($fast = true);

        if ($groupBy) {
            $grouped = [];

            /** @var Player $player */
            foreach ($players as $player) {
                $key = '';

                if ($groupBy == 'country') {
                    $key = $player->getCountry()->getName();
                } elseif ($groupBy == 'team') {
                    $key = $player->getTeam()->getEscapedName();

                    if ($key == '<em>None</em>') {
                        $key = ' ';
                    }
                } elseif ($groupBy == 'activity') {
                    $key = ($player->getMatchActivity() > 0.0) ? 'Active' : 'Inactive';
                }

                $grouped[$key][] = $player;
            }

            ksort($grouped);
            $players = $grouped;
        }

        return array(
            'grouped' => ($groupBy !== null),
            'players' => $players,
        );
    }

    /**
     * Handle the admin notes form
     * @param  Form   $form   The form
     * @param  Player $player The player in question
     * @param  Player $me     The currently logged in player
     * @return Form   The updated form
     */
    private function handleAdminNotesForm($form, $player, $me)
    {
        $notes = $form->get('notes')->getData();
        if ($form->get('save_and_sign')->isClicked()) {
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements 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.
    }
}
            $notes .= ' — ' . $me->getUsername() . ' on ' . TimeDate::now()->toRFC2822String();
        }

        $player->setAdminNotes($notes);
        $this->getFlashBag()->add('success', "The admin notes for {$player->getUsername()} have been updated");

        // Reset the form so that the user sees the updated admin notes
        return $this->creator->create();
    }

    private function handleSorting(&$playerList, $sortBy, $sortOrder)

    {
        if ($sortBy == 'callsign') {
            usort($playerList, function($a, $b) use ($sortOrder) {
                if ($sortOrder == 'DESC') {
                    return strcasecmp($b->getUsername(), $a->getUsername());
                }

                return strcasecmp($a->getUsername(), $b->getUsername());
            });
        } elseif ($sortBy == 'activity') {
            usort($playerList, function($a, $b) use ($sortOrder) {
                if ($sortOrder == 'DESC') {
                    return ($b->getMatchActivity() > $a->getMatchActivity());
                }

                return ($a->getMatchActivity() > $b->getMatchActivity());
            });
        }
    }
}


1			<?php
2
3			use BZIon\Form\Creator\PlayerAdminNotesFormCreator as FormCreator;
4			use Symfony\Component\Form\Form;
5			use Symfony\Component\HttpFoundation\Request;
6
7			class PlayerController extends JSONController
8			{
9			private $creator;
10
11			public function showAction(Player $player, Player $me, Request $request)
12			{
13			if ($me->hasPermission(Permission::VIEW_VISITOR_LOG)) {
14			$this->creator = new FormCreator($player);
15			$form = $this->creator->create()->handleRequest($request);
16
17			if ($form->isValid()) {
18			$form = $this->handleAdminNotesForm($form, $player, $me);
			0 ignored issues – show Compatibility introduced 2016-08-22 14:49 UTC by Report Bug Copy Issue Report `$form` of type `object<Symfony\Component\Form\FormInterface>` is not a sub-type of `object<Symfony\Component\Form\Form>`. It seems like you assume a concrete implementation of the interface `Symfony\Component\Form\FormInterface` to be always present. This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass. Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type. Loading history...
19			}
20
21			$formView = $form->createView();
22			} else {
23			// Don't spend time rendering the form unless we need it
24			$formView = null;
25			}
26
27			return array(
28			"player" => $player,
29			"adminNotesForm" => $formView,
30			);
31			}
32
33			public function editAction(Player $player, Player $me)
34			{
35			if (!$me->canEdit($player)) {
36			throw new ForbiddenException("You are not allowed to edit other players");
37			}
38
39			$params = array(
40			'me' => $player,
41			'self' => false,
42			);
43
44			return $this->forward('edit', $params, 'Profile');
45			}
46
47			public function listAction(Request $request, Player $me, Team $team = null)
48			{
49			$query = Player::getQueryBuilder();
50
51			// Load all countries into the cache so they are ready for later
52			Country::getQueryBuilder()->addToCache();
53
54			if ($team) {
55			$query->where('team')->is($team);
56			} else {
57			// Add all teams to the cache
58			$this->getQueryBuilder('Team')
59			->where('members')->greaterThan(0)
60			->addToCache();
61			}
62
63			if ($request->query->has('exceptMe')) {
64			$query->except($me);
65			}
66
67			$groupBy = $request->query->get('groupBy');
68			$sortBy = $request->query->get('sortBy');
69			$sortOrder = $request->query->get('sortOrder');
70
71			$query->withMatchActivity();
72
73			if ($sortBy \|\| $sortOrder) {
74			$sortBy = $sortBy ? $sortBy : 'callsign';
75			$sortOrder = $sortOrder ? $sortOrder : 'ASC';
76
77			if ($sortBy == 'activity') {
78			$query->sortBy('activity');
79			}
80			elseif ($sortBy == 'callsign') {
81			$query->sortBy('name');
82			}
83
84			if ($sortOrder == 'DESC') {
85			$query->reverse();
86			}
87			}
88
89			$players = $query->getModels($fast = true);
90
91			if ($groupBy) {
92			$grouped = [];
93
94			/** @var Player $player */
95			foreach ($players as $player) {
96			$key = '';
97
98			if ($groupBy == 'country') {
99			$key = $player->getCountry()->getName();
100			} elseif ($groupBy == 'team') {
101			$key = $player->getTeam()->getEscapedName();
102
103			if ($key == '<em>None</em>') {
104			$key = ' ';
105			}
106			} elseif ($groupBy == 'activity') {
107			$key = ($player->getMatchActivity() > 0.0) ? 'Active' : 'Inactive';
108			}
109
110			$grouped[$key][] = $player;
111			}
112
113			ksort($grouped);
114			$players = $grouped;
115			}
116
117			return array(
118			'grouped' => ($groupBy !== null),
119			'players' => $players,
120			);
121			}
122
123			/**
124			* Handle the admin notes form
125			* @param Form $form The form
126			* @param Player $player The player in question
127			* @param Player $me The currently logged in player
128			* @return Form The updated form
129			*/
130			private function handleAdminNotesForm($form, $player, $me)
131			{
132			$notes = $form->get('notes')->getData();
133			if ($form->get('save_and_sign')->isClicked()) {
			0 ignored issues – show Bug introduced 2015-12-02 06:58 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Symfony\Component\Form\FormInterface` as the method `isClicked()` does only exist in the following implementations of said interface: `Symfony\Component\Form\SubmitButton`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements 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 implementation 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 interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
134			$notes .= ' — ' . $me->getUsername() . ' on ' . TimeDate::now()->toRFC2822String();
135			}
136
137			$player->setAdminNotes($notes);
138			$this->getFlashBag()->add('success', "The admin notes for {$player->getUsername()} have been updated");
139
140			// Reset the form so that the user sees the updated admin notes
141			return $this->creator->create();
142			}
143
144			private function handleSorting(&$playerList, $sortBy, $sortOrder)
			0 ignored issues – show Unused Code introduced 2017-07-21 02:46 UTC by Report Bug Copy Issue Report This method is not used, and could be removed. Loading history...
145			{
146			if ($sortBy == 'callsign') {
147			usort($playerList, function($a, $b) use ($sortOrder) {
148			if ($sortOrder == 'DESC') {
149			return strcasecmp($b->getUsername(), $a->getUsername());
150			}
151
152			return strcasecmp($a->getUsername(), $b->getUsername());
153			});
154			} elseif ($sortBy == 'activity') {
155			usort($playerList, function($a, $b) use ($sortOrder) {
156			if ($sortOrder == 'DESC') {
157			return ($b->getMatchActivity() > $a->getMatchActivity());
158			}
159
160			return ($a->getMatchActivity() > $b->getMatchActivity());
161			});
162			}
163			}
164			}
165

allejo / bzion

Push — feature/player-list ( ee00f1...3478af )

PlayerController::listAction() D

Complexity

Size

Duplication

Code Coverage

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Duplication Side-by-Side

Filter issues like