MatchController::listAction() - Code Metrics - Inspection of "Implement team activities" - allejo/bzion - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 466821...a1a5c2 )

by Konstantinos

created 2016-02-20 23:48 UTC

MatchController::listAction() B

↳ Parent: MatchController

Complexity

Conditions	2
Paths	2

Size

Total Lines	24
Code Lines	14

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	13
CRAP Score	2

Metric	Value
dl	0
loc	24
ccs	13
cts	13
cp	1
rs	8.9713
cc	2
eloc	14
nc	2
nop	4
crap	2

<?php

use Symfony\Component\Form\FormError;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\StreamedResponse;

class MatchController extends CRUDController
{
    /**
     * Whether the last edited match has had its ELO changed, requiring an ELO
     * recalculation
     *
     * This is useful so that a confirmation form is shown, asking the user if
     * they want to recalculate ELOs
     *
     * @var bool
     */
    public $recalculateNeeded = false;

    public function listAction(Request $request, Player $me, Team $team = null, $type = null)
    {
        $qb = $this->getQueryBuilder();

        $currentPage = $request->query->get('page', 1);

        $query = $qb->sortBy('time')->reverse()
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.
    }
}
               ->with($team, $type)
               ->limit(50)->fromPage($currentPage);

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

        foreach ($matches as $match) {
            // Don't show wrong labels for matches
            $match->getTimestamp()->setTimezone($me->getTimezone());
        }

        return array(
            "matches"     => $matches,
            "team"        => $team,
            "currentPage" => $currentPage,
            "totalPages"  => $qb->countPages()
        );
    }

    public function createAction(Player $me)
    {
        return $this->create($me, function(Match $match) use ($me) {
            if ($me->canEdit($match)

                && (!$match->getTeamA()->isLastMatch($match)
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                || !$match->getTeamB()->isLastMatch($match))
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            ) {
                $url = Service::getGenerator()->generate('match_recalculate', array(
                    'match' => $match->getId(),
                ));

                return new RedirectResponse($url);
            }
        });
    }

    public function deleteAction(Player $me, Match $match)
    {
        return $this->delete($match, $me, function() use ($match, $me) {
            if ($match->getTeamA()->isLastMatch($match)
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                && $match->getTeamB()->isLastMatch($match)) {
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
                $match->resetELOs();
            } elseif ($me->canEdit($match)) {
                $url = Service::getGenerator()->generate('match_recalculate', array(
                    'match' => $match->getId(),
                ));

                return new RedirectResponse($url);
            }
        });
    }

    public function editAction(Player $me, Match $match)

    {
        // TODO: Generating this response is unnecessary
        $response = $this->edit($match, $me, "match");

        if ($this->recalculateNeeded) {
            // Redirect to a confirmation form if we are assigning a new leader
            $url = Service::getGenerator()->generate('match_recalculate', array(
                'match' => $match->getId(),
            ));

            return new RedirectResponse($url);
        }

        return $response;
    }

    public function recalculateAction(Player $me, $match) {
        $match = Match::get($match); // get a match even if it's deleted

        if (!$me->canEdit($match)) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
            throw new ForbiddenException("You are not allowed to edit that match.");
        }

        return $this->showConfirmationForm(function () use ($match) {
            $response = new StreamedResponse();
            $response->headers->set('Content-Type', 'text/plain');
            $response->setCallback(function() use ($match) {
                $this->recalculate($match);
            });
            $response->send();
        }, "Do you want to recalculate ELO history for all teams and matches after the specified match?",
            "ELO history recalculated",
            "Recalculate ELOs",
            null,
            "Match/recalculate.html.twig");
    }

    /**
     * Recalculates match history for all teams and matches
     *
     * Recalculation is done as follows:
     * 1. A match is chosen as a starting point - it's stored old team ELOs are
     *    considered correct
     * 2. Team ELOs are reset to their values at the starting point
     * 3. Each match that occurred since the first specified match has its ELO
     *    recalculated based on the current team values, and the new match data
     *    and team ELOs are stored in the database
     *
     * @param Match $match The first match
     */
    private function recalculate(Match $match) {
        try {
            // Commented out to prevent ridiculously large recalculations
            //set_time_limit(0);

            $query = Match::getQueryBuilder()
                ->where('status')->notEquals('deleted')
                ->where('time')->isAfter($match->getTimestamp(), $inclusive = true)
                ->sortBy('time');

            /** @var Match[] $matches */
            $matches = $query->getModels($fast = true);

            // Send the total count to client-side javascript
            $this->log(count($matches) . "\n");

            // Start a transaction so tables are locked and we don't stay with
            // messed up data if something goes wrong
            Database::getInstance()->startTransaction();

            $teamsReset = [];

            // Reset match teams, in case the selected match is deleted and does
            // not show up in the list of matches to recalculate
            $match->getTeamA()->setElo($match->getTeamAEloOld());
            $match->getTeamB()->setElo($match->getTeamBEloOld());
            $teamsReset[ $match->getTeamA()->getId() ] = true;
            $teamsReset[ $match->getTeamB()->getId() ] = true;

            foreach ($matches as $i => $match) {
                // Reset teams' ELOs if they haven't been reset already
                if (!isset($teamsReset[ $match->getTeamA()->getId() ])) {

                    $teamsReset[ $match->getTeamA()->getId() ] = true;
                    $match->getTeamA()->setElo($match->getTeamAEloOld());
                }
                if (!isset($teamsReset[ $match->getTeamB()->getId() ])) {

                    $teamsReset[ $match->getTeamB()->getId() ] = true;
                    $match->getTeamB()->setElo($match->getTeamBEloOld());
                }

                $match->recalculateElo();

                // Send an update to the client-side javascript, so that a
                // progress bar can be updated
                $this->log("m");
            }
        } catch (Exception $e) {
            Database::getInstance()->rollback();
            Database::getInstance()->finishTransaction();
            throw $e;
        }

        Database::getInstance()->finishTransaction();

        $this->log("\n\nCalculation successful\n");
    }

    /**
     * Echo a string and flush the buffers
     *
     * Useful for streamed AJAX responses
     *
     * @param string $string The string to echo
     */
    private function log($string)
    {
        echo $string;
        ob_flush();
        flush();
    }

    /**
     * {@inheritdoc}
     */
    protected function getMessages($type, $name = '')
    {
        $messages = parent::getMessages($type, $name);

        // Don't show the match info on the successful create/edit message
        foreach ($messages as &$action) {
            foreach ($action as &$status) {
                if (isset($status['named'])) {
                    $status['named'] = $status['unnamed'];
                }
            }
        }

        return $messages;
    }

    protected function validate($form)
    {
        // Make sure that two different teams participated in a match, i.e. a team
        // didn't match against itself
        $firstTeam  = $form->get('first_team')->get('team')->getData();
        $secondTeam = $form->get('second_team')->get('team')->getData();

        if (!$firstTeam || !$secondTeam) {
            return;
        }

        if ($firstTeam->isSameAs($secondTeam)) {
            $message = "You can't report a match where a team played against itself!";
            $form->addError(new FormError($message));
        }
    }
}


1			<?php
2
3			use Symfony\Component\Form\FormError;
4			use Symfony\Component\HttpFoundation\RedirectResponse;
5			use Symfony\Component\HttpFoundation\Request;
6			use Symfony\Component\HttpFoundation\StreamedResponse;
7
8			class MatchController extends CRUDController
9			{
10			/**
11			* Whether the last edited match has had its ELO changed, requiring an ELO
12			* recalculation
13			*
14			* This is useful so that a confirmation form is shown, asking the user if
15			* they want to recalculate ELOs
16			*
17			* @var bool
18			*/
19			public $recalculateNeeded = false;
20
21	1		public function listAction(Request $request, Player $me, Team $team = null, $type = null)
22			{
23	1		$qb = $this->getQueryBuilder();
24
25	1		$currentPage = $request->query->get('page', 1);
26
27	1		$query = $qb->sortBy('time')->reverse()
			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 specific sub-type and not the parent class `QueryBuilder` as the method `with()` does only exist in the following sub-classes of `QueryBuilder`: `MatchQueryBuilder`. 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...
28	1		->with($team, $type)
29	1		->limit(50)->fromPage($currentPage);
30
31			$matches = $query->getModels($fast = true);
32
33	1		foreach ($matches as $match) {
34	1		// Don't show wrong labels for matches
35	1		$match->getTimestamp()->setTimezone($me->getTimezone());
36	1		}
37
38			return array(
39			"matches" => $matches,
40	1		"team" => $team,
41			"currentPage" => $currentPage,
42			"totalPages" => $qb->countPages()
43	1		);
44	1		}
45	1
46			public function createAction(Player $me)
47			{
48			return $this->create($me, function(Match $match) use ($me) {
49		View Code Duplication	if ($me->canEdit($match)
			0 ignored issues – show Duplication introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
50			&& (!$match->getTeamA()->isLastMatch($match)
			0 ignored issues – show Documentation introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report `$match` is of type `object<Match>`, but the function expects a `integer`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
51			\|\| !$match->getTeamB()->isLastMatch($match))
			0 ignored issues – show Documentation introduced 2015-12-02 06:58 UTC by Report Bug Copy Issue Report `$match` is of type `object<Match>`, but the function expects a `integer`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
52			) {
53	1		$url = Service::getGenerator()->generate('match_recalculate', array(
54			'match' => $match->getId(),
55			));
56
57			return new RedirectResponse($url);
58			}
59			});
60			}
61
62			public function deleteAction(Player $me, Match $match)
63			{
64			return $this->delete($match, $me, function() use ($match, $me) {
65		View Code Duplication	if ($match->getTeamA()->isLastMatch($match)
			0 ignored issues – show Documentation introduced 2015-12-02 06:58 UTC by Report Bug Copy Issue Report `$match` is of type `object<Match>`, but the function expects a `integer`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history... Duplication introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
66			&& $match->getTeamB()->isLastMatch($match)) {
			0 ignored issues – show Documentation introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report `$match` is of type `object<Match>`, but the function expects a `integer`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
67			$match->resetELOs();
68			} elseif ($me->canEdit($match)) {
69			$url = Service::getGenerator()->generate('match_recalculate', array(
70			'match' => $match->getId(),
71			));
72
73			return new RedirectResponse($url);
74			}
75			});
76			}
77
78		View Code Duplication	public function editAction(Player $me, Match $match)
			0 ignored issues – show Duplication introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
79			{
80			// TODO: Generating this response is unnecessary
81			$response = $this->edit($match, $me, "match");
82
83			if ($this->recalculateNeeded) {
84			// Redirect to a confirmation form if we are assigning a new leader
85			$url = Service::getGenerator()->generate('match_recalculate', array(
86			'match' => $match->getId(),
87			));
88
89			return new RedirectResponse($url);
90			}
91
92			return $response;
93			}
94
95			public function recalculateAction(Player $me, $match) {
96			$match = Match::get($match); // get a match even if it's deleted
97
98			if (!$me->canEdit($match)) {
			0 ignored issues – show Bug introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report It seems like `$match` defined by `\Match::get($match)` on line `96` can be `null`; however, `Player::canEdit()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
99			throw new ForbiddenException("You are not allowed to edit that match.");
100			}
101
102			return $this->showConfirmationForm(function () use ($match) {
103			$response = new StreamedResponse();
104			$response->headers->set('Content-Type', 'text/plain');
105			$response->setCallback(function() use ($match) {
106			$this->recalculate($match);
107			});
108			$response->send();
109			}, "Do you want to recalculate ELO history for all teams and matches after the specified match?",
110			"ELO history recalculated",
111			"Recalculate ELOs",
112			null,
113			"Match/recalculate.html.twig");
114			}
115
116			/**
117			* Recalculates match history for all teams and matches
118			*
119			* Recalculation is done as follows:
120			* 1. A match is chosen as a starting point - it's stored old team ELOs are
121			* considered correct
122			* 2. Team ELOs are reset to their values at the starting point
123			* 3. Each match that occurred since the first specified match has its ELO
124			* recalculated based on the current team values, and the new match data
125			* and team ELOs are stored in the database
126			*
127			* @param Match $match The first match
128			*/
129			private function recalculate(Match $match) {
130			try {
131			// Commented out to prevent ridiculously large recalculations
132			//set_time_limit(0);
133
134			$query = Match::getQueryBuilder()
135			->where('status')->notEquals('deleted')
136			->where('time')->isAfter($match->getTimestamp(), $inclusive = true)
137			->sortBy('time');
138
139			/** @var Match[] $matches */
140			$matches = $query->getModels($fast = true);
141
142			// Send the total count to client-side javascript
143			$this->log(count($matches) . "\n");
144
145			// Start a transaction so tables are locked and we don't stay with
146			// messed up data if something goes wrong
147			Database::getInstance()->startTransaction();
148
149			$teamsReset = [];
150
151			// Reset match teams, in case the selected match is deleted and does
152			// not show up in the list of matches to recalculate
153			$match->getTeamA()->setElo($match->getTeamAEloOld());
154			$match->getTeamB()->setElo($match->getTeamBEloOld());
155			$teamsReset[ $match->getTeamA()->getId() ] = true;
156			$teamsReset[ $match->getTeamB()->getId() ] = true;
157
158			foreach ($matches as $i => $match) {
159			// Reset teams' ELOs if they haven't been reset already
160		View Code Duplication	if (!isset($teamsReset[ $match->getTeamA()->getId() ])) {
			0 ignored issues – show Duplication introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
161			$teamsReset[ $match->getTeamA()->getId() ] = true;
162			$match->getTeamA()->setElo($match->getTeamAEloOld());
163			}
164		View Code Duplication	if (!isset($teamsReset[ $match->getTeamB()->getId() ])) {
			0 ignored issues – show Duplication introduced 2016-02-20 00:57 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
165			$teamsReset[ $match->getTeamB()->getId() ] = true;
166			$match->getTeamB()->setElo($match->getTeamBEloOld());
167			}
168
169			$match->recalculateElo();
170
171			// Send an update to the client-side javascript, so that a
172			// progress bar can be updated
173			$this->log("m");
174			}
175			} catch (Exception $e) {
176			Database::getInstance()->rollback();
177			Database::getInstance()->finishTransaction();
178			throw $e;
179			}
180
181			Database::getInstance()->finishTransaction();
182
183			$this->log("\n\nCalculation successful\n");
184			}
185
186			/**
187			* Echo a string and flush the buffers
188			*
189			* Useful for streamed AJAX responses
190			*
191			* @param string $string The string to echo
192			*/
193			private function log($string)
194			{
195			echo $string;
196			ob_flush();
197	1		flush();
198			}
199	1
200			/**
201			* {@inheritdoc}
202	1		*/
203	1		protected function getMessages($type, $name = '')
204	1		{
205	1		$messages = parent::getMessages($type, $name);
206
207			// Don't show the match info on the successful create/edit message
208			foreach ($messages as &$action) {
209			foreach ($action as &$status) {
210	1		if (isset($status['named'])) {
211			$status['named'] = $status['unnamed'];
212			}
213	1		}
214			}
215
216			return $messages;
217	1		}
218	1
219			protected function validate($form)
220	1		{
221			// Make sure that two different teams participated in a match, i.e. a team
222			// didn't match against itself
223			$firstTeam = $form->get('first_team')->get('team')->getData();
224	1		$secondTeam = $form->get('second_team')->get('team')->getData();
225	1
226	1		if (!$firstTeam \|\| !$secondTeam) {
227			return;
228	1		}
229
230			if ($firstTeam->isSameAs($secondTeam)) {
231			$message = "You can't report a match where a team played against itself!";
232			$form->addError(new FormError($message));
233			}
234			}
235			}
236

allejo / bzion

Push — master ( 466821...a1a5c2 )

MatchController::listAction() B

Complexity

Size

Duplication

Code Coverage

Available Fixes

Duplication Side-by-Side

Filter issues like