Passed
Push — dev ( 7914d2...bbc331 )
by Janko
10:29
created

AllianceDeletionHandler::delete()   B

Complexity

Conditions 10
Paths 14

Size

Total Lines 42
Code Lines 29

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 23
CRAP Score 11.2698

Importance

Changes 1
Bugs 1 Features 0
Metric Value
cc 10
eloc 29
nc 14
nop 1
dl 0
loc 42
ccs 23
cts 30
cp 0.7667
crap 11.2698
rs 7.6666
c 1
b 1
f 0

How to fix   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
declare(strict_types=1);
4
5
namespace Stu\Component\Player\Deletion\Handler;
6
7
use Override;
0 ignored issues
show
Bug introduced by
The type Override was not found. Maybe you did not declare it correctly or list all dependencies?

The issue could also be caused by a filter entry in the build configuration. If the path has been excluded in your configuration, e.g. excluded_paths: ["lib/*"], you can move it to the dependency path list as follows:

filter:
    dependency_paths: ["lib/*"]

For further information see https://scrutinizer-ci.com/docs/tools/php/php-scrutinizer/#list-dependency-paths

Loading history...
8
use Doctrine\Common\Collections\Collection;
9
use Stu\Component\Alliance\AllianceEnum;
0 ignored issues
show
Bug introduced by
The type Stu\Component\Alliance\AllianceEnum was not found. Maybe you did not declare it correctly or list all dependencies?

The issue could also be caused by a filter entry in the build configuration. If the path has been excluded in your configuration, e.g. excluded_paths: ["lib/*"], you can move it to the dependency path list as follows:

filter:
    dependency_paths: ["lib/*"]

For further information see https://scrutinizer-ci.com/docs/tools/php/php-scrutinizer/#list-dependency-paths

Loading history...
10
use Stu\Module\Alliance\Lib\AllianceActionManagerInterface;
11
use Stu\Orm\Entity\User;
12
use Stu\Orm\Repository\AllianceJobRepositoryInterface;
13
use Stu\Orm\Repository\UserRepositoryInterface;
14
15
final class AllianceDeletionHandler implements PlayerDeletionHandlerInterface
16
{
17 4
    public function __construct(private AllianceJobRepositoryInterface $allianceJobRepository, private AllianceActionManagerInterface $allianceActionManager, private UserRepositoryInterface $userRepository) {}
18
19 3
    #[Override]
20
    public function delete(User $user): void
21
    {
22 3
        foreach ($this->allianceJobRepository->getByUser($user->getId()) as $job) {
23 3
            if ($job->getType() === AllianceEnum::ALLIANCE_JOBS_FOUNDER) {
24
25 2
                $alliance = $job->getAlliance();
26 2
                $successor = $alliance->getSuccessor();
27 2
                $diplomatic = $alliance->getDiplomatic();
28
29 2
                $members = $alliance->getMembers();
30 2
                $members->removeElement($user);
31 2
                $user->setAlliance(null);
32 2
                $this->userRepository->save($user);
33
34 2
                $lastonlinemember = $this->getLastOnlineMember($members);
0 ignored issues
show
Bug introduced by
Are you sure the assignment to $lastonlinemember is correct as $this->getLastOnlineMember($members) targeting Stu\Component\Player\Del...::getLastOnlineMember() seems to always return null.

This check looks for function or method calls that always return null and whose return value is assigned to a variable.

class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
$object = $a->getObject();

The method getObject() can return nothing but null, so it makes no sense to assign that value to a variable.

The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.

Loading history...
35
36 2
                if ($successor === null && $lastonlinemember === null) {
37 1
                    $this->allianceJobRepository->delete($job);
38 1
                    $this->allianceActionManager->delete($alliance);
39
                }
40 2
                if ($successor !== null) {
41
42 1
                    $this->allianceActionManager->setJobForUser(
43 1
                        $alliance,
44 1
                        $successor->getUser(),
45 1
                        AllianceEnum::ALLIANCE_JOBS_FOUNDER
46 1
                    );
47 1
                    $this->allianceJobRepository->delete($successor);
48
                }
49 2
                if ($successor == null && $lastonlinemember != null) {
50
                    if ($diplomatic !== null && $lastonlinemember == $diplomatic->getUser()) {
51
                        $this->allianceJobRepository->delete($diplomatic);
52
                    }
53
                    $this->allianceActionManager->setJobForUser(
54
                        $alliance,
55
                        $lastonlinemember,
0 ignored issues
show
Bug introduced by
$lastonlinemember of type null is incompatible with the type Stu\Orm\Entity\User expected by parameter $user of Stu\Module\Alliance\Lib\...erface::setJobForUser(). ( Ignorable by Annotation )

If this is a false-positive, you can also ignore this issue in your code via the ignore-type  annotation

55
                        /** @scrutinizer ignore-type */ $lastonlinemember,
Loading history...
56
                        AllianceEnum::ALLIANCE_JOBS_FOUNDER
57
                    );
58
                }
59
            } else {
60 1
                $this->allianceJobRepository->delete($job);
61
            }
62
        }
63
    }
64
65
    /**
66
     * @param Collection<int, User> $members
67
     */
68 2
    private function getLastOnlineMember(Collection $members): ?User
69
    {
70 2
        $lastOnlineMember = null;
71 2
        $maxLastAction = 0;
72
73 2
        foreach ($members as $member) {
74
            if ($member->getLastAction() > $maxLastAction) {
75
                $maxLastAction = $member->getLastAction();
76
                $lastOnlineMember = $member;
77
            }
78
        }
79
80 2
        return $lastOnlineMember;
81
    }
82
}
83