MatchController::getMessages() - Code Metrics - Inspection of "Add support for editing and deleting arbitrary mat..." - allejo/bzion - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( bce3ff...b78e58 )

by Konstantinos

created 2016-02-20 00:53 UTC

MatchController::getMessages() A

↳ Parent: MatchController

Complexity

Conditions	4
Paths	4

Size

Total Lines	15
Code Lines	7

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	7
CRAP Score	4

Metric	Value
dl	0
loc	15
ccs	7
cts	7
cp	1
rs	9.2
cc	4
eloc	7
nc	4
nop	2
crap	4

<?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, 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);


        return array(
            "matches"     => $query->getModels($fast = true),
            "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, 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
32			return array(
33	1		"matches" => $query->getModels($fast = true),
34	1		"team" => $team,
35	1		"currentPage" => $currentPage,
36	1		"totalPages" => $qb->countPages()
37			);
38			}
39
40	1		public function createAction(Player $me)
41			{
42			return $this->create($me, function(Match $match) use ($me) {
43	1	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...
44	1		&& (!$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...
45	1		\|\| !$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...
46			) {
47			$url = Service::getGenerator()->generate('match_recalculate', array(
48			'match' => $match->getId(),
49			));
50
51			return new RedirectResponse($url);
52			}
53	1		});
54			}
55
56			public function deleteAction(Player $me, Match $match)
57			{
58			return $this->delete($match, $me, function() use ($match, $me) {
59		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...
60			&& $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...
61			$match->resetELOs();
62			} elseif ($me->canEdit($match)) {
63			$url = Service::getGenerator()->generate('match_recalculate', array(
64			'match' => $match->getId(),
65			));
66
67			return new RedirectResponse($url);
68			}
69			});
70			}
71
72		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...
73			{
74			// TODO: Generating this response is unnecessary
75			$response = $this->edit($match, $me, "match");
76
77			if ($this->recalculateNeeded) {
78			// Redirect to a confirmation form if we are assigning a new leader
79			$url = Service::getGenerator()->generate('match_recalculate', array(
80			'match' => $match->getId(),
81			));
82
83			return new RedirectResponse($url);
84			}
85
86			return $response;
87			}
88
89			public function recalculateAction(Player $me, $match) {
90			$match = Match::get($match); // get a match even if it's deleted
91
92			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 `90` 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...
93			throw new ForbiddenException("You are not allowed to edit that match.");
94			}
95
96			return $this->showConfirmationForm(function () use ($match) {
97			$response = new StreamedResponse();
98			$response->headers->set('Content-Type', 'text/plain');
99			$response->setCallback(function() use ($match) {
100			$this->recalculate($match);
101			});
102			$response->send();
103			}, "Do you want to recalculate ELO history for all teams and matches after the specified match?",
104			"ELO history recalculated",
105			"Recalculate ELOs",
106			null,
107			"Match/recalculate.html.twig");
108			}
109
110			/**
111			* Recalculates match history for all teams and matches
112			*
113			* Recalculation is done as follows:
114			* 1. A match is chosen as a starting point - it's stored old team ELOs are
115			* considered correct
116			* 2. Team ELOs are reset to their values at the starting point
117			* 3. Each match that occurred since the first specified match has its ELO
118			* recalculated based on the current team values, and the new match data
119			* and team ELOs are stored in the database
120			*
121			* @param Match $match The first match
122			*/
123			private function recalculate(Match $match) {
124			try {
125			// Commented out to prevent ridiculously large recalculations
126			//set_time_limit(0);
127
128			$query = Match::getQueryBuilder()
129			->where('status')->notEquals('deleted')
130			->where('time')->isAfter($match->getTimestamp(), $inclusive = true)
131			->sortBy('time');
132
133			/** @var Match[] $matches */
134			$matches = $query->getModels($fast = true);
135
136			// Send the total count to client-side javascript
137			$this->log(count($matches) . "\n");
138
139			// Start a transaction so tables are locked and we don't stay with
140			// messed up data if something goes wrong
141			Database::getInstance()->startTransaction();
142
143			$teamsReset = [];
144
145			// Reset match teams, in case the selected match is deleted and does
146			// not show up in the list of matches to recalculate
147			$match->getTeamA()->setElo($match->getTeamAEloOld());
148			$match->getTeamB()->setElo($match->getTeamBEloOld());
149			$teamsReset[ $match->getTeamA()->getId() ] = true;
150			$teamsReset[ $match->getTeamB()->getId() ] = true;
151
152			foreach ($matches as $i => $match) {
153			// Reset teams' ELOs if they haven't been reset already
154		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...
155			$teamsReset[ $match->getTeamA()->getId() ] = true;
156			$match->getTeamA()->setElo($match->getTeamAEloOld());
157			}
158		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...
159			$teamsReset[ $match->getTeamB()->getId() ] = true;
160			$match->getTeamB()->setElo($match->getTeamBEloOld());
161			}
162
163			$match->recalculateElo();
164
165			// Send an update to the client-side javascript, so that a
166			// progress bar can be updated
167			$this->log("m");
168			}
169			} catch (Exception $e) {
170			Database::getInstance()->rollback();
171			Database::getInstance()->finishTransaction();
172			throw $e;
173			}
174
175			Database::getInstance()->finishTransaction();
176
177			$this->log("\n\nCalculation successful\n");
178			}
179
180			/**
181			* Echo a string and flush the buffers
182			*
183			* Useful for streamed AJAX responses
184			*
185			* @param string $string The string to echo
186			*/
187			private function log($string)
188			{
189			echo $string;
190			ob_flush();
191			flush();
192			}
193
194			/**
195			* {@inheritdoc}
196			*/
197	1		protected function getMessages($type, $name = '')
198			{
199	1		$messages = parent::getMessages($type, $name);
200
201			// Don't show the match info on the successful create/edit message
202	1		foreach ($messages as &$action) {
203	1		foreach ($action as &$status) {
204	1		if (isset($status['named'])) {
205	1		$status['named'] = $status['unnamed'];
206			}
207			}
208			}
209
210	1		return $messages;
211			}
212
213	1		protected function validate($form)
214			{
215			// Make sure that two different teams participated in a match, i.e. a team
216			// didn't match against itself
217	1		$firstTeam = $form->get('first_team')->get('team')->getData();
218	1		$secondTeam = $form->get('second_team')->get('team')->getData();
219
220	1		if (!$firstTeam \|\| !$secondTeam) {
221			return;
222			}
223
224	1		if ($firstTeam->isSameAs($secondTeam)) {
225	1		$message = "You can't report a match where a team played against itself!";
226	1		$form->addError(new FormError($message));
227			}
228	1		}
229			}
230

allejo / bzion

Push — master ( bce3ff...b78e58 )

MatchController::getMessages() A

Complexity

Size

Duplication

Code Coverage

Available Fixes

Duplication Side-by-Side

Filter issues like