PlayerController::listAction() - Code Metrics - Inspection of "Fix lazy loading for new modals; break old models" - allejo/bzion - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — feature/pixie-port ( 8213a1...132a2b )

by Vladimir

created 2018-02-14 08:47 UTC

PlayerController::listAction() C

↳ Parent: PlayerController

Complexity

Conditions	15
Paths	80

Size

Total Lines	75
Code Lines	43

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	75
rs	5.3122
c	0
b	0
f	0
cc	15
eloc	43
nc	80
nop	3

How to fix Long Method Complexity

<?php

use BZIon\Form\Creator\PlayerAdminNotesFormCreator as FormCreator;
use Carbon\Carbon;
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)
    {
        $formView = null;

        if ($me->hasPermission(Permission::VIEW_VISITOR_LOG) && !$this->isDemoMode()) {
            $this->creator = new FormCreator($player);
            $form = $this->creator->create()->handleRequest($request);

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

            $formView = $form->createView();
        }

        $periods = [];
        $season = Season::getCurrentSeasonRange();
        $periodLength = round($season->getEndOfRange()->diffInDays($season->getStartOfRange()) / 30);
        $seasonStart = $season->getStartOfRange();

        for ($i = 0; $i < $periodLength; $i++) {
            $periods[] = $seasonStart->firstOfMonth()->copy();
            $periods[] = $seasonStart->day(15)->copy();
            $periods[] = $seasonStart->lastOfMonth()->copy();

            $seasonStart->addMonth();
        }

        $currentPeriod = new ArrayIterator($periods);
        $playerEloSeason = $player->getEloSeasonHistory();
        $seasonSummary = [];

        foreach ($playerEloSeason as $elo) {
            if ($elo['month'] > $currentPeriod->current()->month || $elo['day'] > $currentPeriod->current()->day) {
                $currentPeriod->next();
            }

            $seasonSummary[$currentPeriod->current()->format('M d')] = $elo['elo'];
        }

        $bans = Ban::getQueryBuilder()
            ->where('player', '=', $player->getId())
            ->getModels($fast = true)
        ;

        return array(
            'bans' => $bans,
            'player'         => $player,
            'seasonSummary'  => $seasonSummary,
            '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', '=', $team);
        } else {
            // Add all teams to the cache
            Team::getQueryBuilder()
                ->where('members', '>', 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
            ->active()
            ->withMatchActivity()
            ->orderBy('username')
        ;

        if (!$request->query->get('showAll')) {
            $query->having('activity', '>', 0);
        }

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

            $query->orderBy($sortBy, $sortOrder);
        }

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

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

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

                if ($groupBy == 'country') {
                    $key = $player->getCountry()->getName();
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.
    }
}
                } elseif ($groupBy == 'team') {
                    $key = $player->getTeam()->getEscapedName();
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.
    }
}

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

                $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()) {

            $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();
    }
}


1			<?php
2
3			use BZIon\Form\Creator\PlayerAdminNotesFormCreator as FormCreator;
4			use Carbon\Carbon;
5			use Symfony\Component\Form\Form;
6			use Symfony\Component\HttpFoundation\Request;
7
8			class PlayerController extends JSONController
9			{
10			private $creator;
11
12			public function showAction(Player $player, Player $me, Request $request)
13			{
14			$formView = null;
15
16			if ($me->hasPermission(Permission::VIEW_VISITOR_LOG) && !$this->isDemoMode()) {
17			$this->creator = new FormCreator($player);
18			$form = $this->creator->create()->handleRequest($request);
19
20			if ($form->isValid()) {
21			$form = $this->handleAdminNotesForm($form, $player, $me);
22			}
23
24			$formView = $form->createView();
25			}
26
27			$periods = [];
28			$season = Season::getCurrentSeasonRange();
29			$periodLength = round($season->getEndOfRange()->diffInDays($season->getStartOfRange()) / 30);
30			$seasonStart = $season->getStartOfRange();
31
32			for ($i = 0; $i < $periodLength; $i++) {
33			$periods[] = $seasonStart->firstOfMonth()->copy();
34			$periods[] = $seasonStart->day(15)->copy();
35			$periods[] = $seasonStart->lastOfMonth()->copy();
36
37			$seasonStart->addMonth();
38			}
39
40			$currentPeriod = new ArrayIterator($periods);
41			$playerEloSeason = $player->getEloSeasonHistory();
42			$seasonSummary = [];
43
44			foreach ($playerEloSeason as $elo) {
45			if ($elo['month'] > $currentPeriod->current()->month \|\| $elo['day'] > $currentPeriod->current()->day) {
46			$currentPeriod->next();
47			}
48
49			$seasonSummary[$currentPeriod->current()->format('M d')] = $elo['elo'];
50			}
51
52			$bans = Ban::getQueryBuilder()
53			->where('player', '=', $player->getId())
54			->getModels($fast = true)
55			;
56
57			return array(
58			'bans' => $bans,
59			'player' => $player,
60			'seasonSummary' => $seasonSummary,
61			'adminNotesForm' => $formView,
62			);
63			}
64
65			public function editAction(Player $player, Player $me)
66			{
67			if (!$me->canEdit($player)) {
68			throw new ForbiddenException("You are not allowed to edit other players");
69			}
70
71			$params = array(
72			'me' => $player,
73			'self' => false,
74			);
75
76			return $this->forward('edit', $params, 'Profile');
77			}
78
79			public function listAction(Request $request, Player $me, Team $team = null)
80			{
81			$query = Player::getQueryBuilder();
82
83			// Load all countries into the cache so they are ready for later
84			Country::getQueryBuilder()->addToCache();
85
86			if ($team) {
87			$query->where('team', '=', $team);
88			} else {
89			// Add all teams to the cache
90			Team::getQueryBuilder()
91			->where('members', '>', 0)
92			->addToCache()
93			;
94			}
95
96			if ($request->query->has('exceptMe')) {
97			$query->except($me);
98			}
99
100			$groupBy = $request->query->get('groupBy');
101			$sortBy = $request->query->get('sortBy');
102			$sortOrder = $request->query->get('sortOrder');
103
104			$query
105			->active()
106			->withMatchActivity()
107			->orderBy('username')
108			;
109
110			if (!$request->query->get('showAll')) {
111			$query->having('activity', '>', 0);
112			}
113
114			if ($sortBy \|\| $sortOrder) {
115			$sortBy = $sortBy ? 'activity' : 'callsign';
116			$sortOrder = $sortOrder ? 'DESC' : 'ASC';
117
118			$query->orderBy($sortBy, $sortOrder);
119			}
120
121			$players = $query->getModels($fast = true);
122
123			if ($groupBy) {
124			$grouped = [];
125
126			/** @var Player $player */
127			foreach ($players as $player) {
128			$key = '';
129
130			if ($groupBy == 'country') {
131			$key = $player->getCountry()->getName();
			0 ignored issues – show Bug introduced 2018-02-13 08:59 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Model` as the method `getCountry()` does only exist in the following sub-classes of `Model`: `Player`, `Server`. 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...
132			} elseif ($groupBy == 'team') {
133			$key = $player->getTeam()->getEscapedName();
			0 ignored issues – show Bug introduced 2018-02-13 08:59 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Model` as the method `getTeam()` does only exist in the following sub-classes of `Model`: `Invitation`, `Player`. 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...
134
135			if ($key == '<em>None</em>') {
136			$key = ' ';
137			}
138			} elseif ($groupBy == 'activity') {
139			$key = ($player->getMatchActivity() > 0.0) ? 'Active' : 'Inactive';
			0 ignored issues – show Bug introduced 2018-02-13 08:59 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Model` as the method `getMatchActivity()` does only exist in the following sub-classes of `Model`: `Player`. 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...
140			}
141
142			$grouped[$key][] = $player;
143			}
144
145			ksort($grouped);
146			$players = $grouped;
147			}
148
149			return array(
150			'grouped' => ($groupBy !== null),
151			'players' => $players,
152			);
153			}
154
155			/**
156			* Handle the admin notes form
157			* @param Form $form The form
158			* @param Player $player The player in question
159			* @param Player $me The currently logged in player
160			* @return Form The updated form
161			*/
162			private function handleAdminNotesForm($form, $player, $me)
163			{
164			$notes = $form->get('notes')->getData();
165			if ($form->get('save_and_sign')->isClicked()) {
			0 ignored issues – show Bug introduced 2017-09-08 23:57 UTC by Report Bug Copy Issue Report The method `isClicked()` does not seem to exist on `object<Symfony\Component\Form\Form>`. This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces. This is most likely a typographical error or the method has been renamed. Loading history...
166			$notes .= ' — ' . $me->getUsername() . ' on ' . TimeDate::now()->toRFC2822String();
167			}
168
169			$player->setAdminNotes($notes);
170			$this->getFlashBag()->add('success', "The admin notes for {$player->getUsername()} have been updated");
171
172			// Reset the form so that the user sees the updated admin notes
173			return $this->creator->create();
174			}
175			}
176

allejo / bzion

Push — feature/pixie-port ( 8213a1...132a2b )

PlayerController::listAction() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like