Completed
Push — feature/player-list ( 8e5225...c69c4b )
by Vladimir
05:41
created

PlayerController::listAction()   F

Complexity

Conditions 17
Paths 272

Size

Total Lines 80
Code Lines 46

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 0
CRAP Score 306

Importance

Changes 0
Metric Value
dl 0
loc 80
ccs 0
cts 43
cp 0
rs 3.8247
c 0
b 0
f 0
cc 17
eloc 46
nc 272
nop 3
crap 306

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

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 by
$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
72
            ->withMatchActivity()
73
            ->sortBy('name')
74
            ->active()
75
        ;
76
77
        if (!$request->query->get('showAll')) {
78
            $query->having('activity')->greaterThan(0);
79
        }
80
81
        if ($sortBy || $sortOrder) {
82
            $sortBy = $sortBy ? $sortBy : 'callsign';
83
            $sortOrder = $sortOrder ? $sortOrder : 'ASC';
84
85
            if ($sortBy === 'activity') {
86
                $query->sortBy($sortBy);
87
            }
88
89
            if ($sortOrder == 'DESC') {
90
                $query->reverse();
91
            }
92
        }
93
94
        $players = $query->getModels($fast = true);
95
96
        if ($groupBy) {
97
            $grouped = [];
98
99
            /** @var Player $player */
100
            foreach ($players as $player) {
101
                $key = '';
102
103
                if ($groupBy == 'country') {
104
                    $key = $player->getCountry()->getName();
105
                } elseif ($groupBy == 'team') {
106
                    $key = $player->getTeam()->getEscapedName();
107
108
                    if ($key == '<em>None</em>') {
109
                        $key = ' ';
110
                    }
111
                } elseif ($groupBy == 'activity') {
112
                    $key = ($player->getMatchActivity() > 0.0) ? 'Active' : 'Inactive';
113
                }
114
115
                $grouped[$key][] = $player;
116
            }
117
118
            ksort($grouped);
119
            $players = $grouped;
120
        }
121
122
        return array(
123
            'grouped' => ($groupBy !== null),
124
            'players' => $players,
125
        );
126
    }
127
128
    /**
129
     * Handle the admin notes form
130
     * @param  Form   $form   The form
131
     * @param  Player $player The player in question
132
     * @param  Player $me     The currently logged in player
133
     * @return Form   The updated form
134
     */
135
    private function handleAdminNotesForm($form, $player, $me)
136
    {
137
        $notes = $form->get('notes')->getData();
138
        if ($form->get('save_and_sign')->isClicked()) {
0 ignored issues
show
Bug introduced by
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

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. 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.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
139
            $notes .= ' — ' . $me->getUsername() . ' on ' . TimeDate::now()->toRFC2822String();
140
        }
141
142
        $player->setAdminNotes($notes);
143
        $this->getFlashBag()->add('success', "The admin notes for {$player->getUsername()} have been updated");
144
145
        // Reset the form so that the user sees the updated admin notes
146
        return $this->creator->create();
147
    }
148
}
149