Completed
Pull Request — master (#123)
by De Cramer
12:37
created

MenuFactory::__construct()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 17
Code Lines 13

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 17
c 0
b 0
f 0
rs 9.4285
cc 1
eloc 13
nc 1
nop 8

How to fix   Many Parameters   

Many Parameters

Methods with many parameters are not only hard to understand, but their parameters also often become inconsistent when you need more, or different data.

There are several approaches to avoid long parameter lists:

1
<?php
2
3
namespace eXpansion\Bundle\Menu\Plugins\Gui;
4
5
use eXpansion\Framework\AdminGroups\Helpers\AdminGroups;
6
use eXpansion\Framework\Core\Model\Gui\ManialinkInterface;
7
use eXpansion\Framework\Core\Model\Gui\ManiaScriptFactory;
8
use eXpansion\Framework\Core\Model\Gui\Widget;
9
use eXpansion\Framework\Core\Model\Gui\WidgetFactoryContext;
10
use eXpansion\Framework\Core\Plugins\Gui\WidgetFactory;
11
use eXpansion\Framework\Gui\Components\uiAnimation;
12
use eXpansion\Framework\Gui\Components\uiButton;
13
use eXpansion\Framework\Gui\Components\uiLabel;
14
use FML\Controls\Frame;
15
use FML\Controls\Label;
16
use FML\Controls\Quad;
17
18
19
/**
20
 * Class MenuFactory
21
 *
22
 * @package eXpansion\Bundle\Menu\Plugins\Gui;
23
 * @author  oliver de Cramer <[email protected]>
24
 */
25
class MenuFactory extends WidgetFactory
26
{
27
    /** @var  ManiaScriptFactory */
28
    protected $menuScriptFactory;
29
30
    /** @var  AdminGroups */
31
    protected $adminGroupsHelper;
32
33
    public $currentMenuView;
34
35
36
    public function __construct(
37
        $name,
38
        $sizeX,
39
        $sizeY,
40
        $posX,
41
        $posY,
42
        WidgetFactoryContext $context,
43
        ManiaScriptFactory $menuScriptFactory,
44
        AdminGroups $adminGroupsHelper
45
    ) {
46
        parent::__construct($name, $sizeX, $sizeY, $posX, $posY, $context);
47
48
        $this->menuScriptFactory = $menuScriptFactory;
49
        $this->adminGroupsHelper = $adminGroupsHelper;
50
        $this->currentMenuView = Frame::create();
51
52
    }
53
54
55
    /**
56
     * @param ManialinkInterface|Widget $manialink
57
     */
58
    protected function createContent(ManialinkInterface $manialink)
59
    {
60
61
    }
62
63
    /**
64
     * @param ManialinkInterface|Widget $manialink
65
     */
66
    protected function updateContent(ManialinkInterface $manialink)
67
    {
68
        // Do stuff Here.
69
        $manialink->removeAllChildren();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface eXpansion\Framework\Core...\Gui\ManialinkInterface as the method removeAllChildren() does only exist in the following implementations of said interface: eXpansion\Framework\Core\Model\Gui\Widget, eXpansion\Framework\Core\Model\Gui\Window.

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...
70
        $manialink->getFmlManialink()->getScript()->addScriptFunction("",
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface eXpansion\Framework\Core...\Gui\ManialinkInterface as the method getFmlManialink() does only exist in the following implementations of said interface: eXpansion\Framework\Core\Model\Gui\Widget, eXpansion\Framework\Core\Model\Gui\Window.

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...
71
            <<<EOL
72
    
73
    Void onButtonOver(CMlControl Element) {
74
        Audio.PlaySoundEvent( CAudioManager::ELibSound::ScoreIncrease, 0, 0.);
75
76
        if (Element is CMlLabel) {
77
                declare El = (Element as CMlLabel);
78
        }
79
    
80
        if (Element.HasClass("noAnim") == False) {
81
            AnimMgr.Add(Element, "<elem scale=\"1.3\" />", 300, CAnimManager::EAnimManagerEasing::ElasticOut);
82
        }
83
    }   
84
85
    Void onButtonOut(CMlControl Element) {
86
        if (Element.HasClass("noAnim") == False) {
87
            AnimMgr.Add(Element, "<elem scale=\"1.0\" />", 300, CAnimManager::EAnimManagerEasing::ElasticOut);
88
		}
89
    }
90
91
92
***FML_OnInit***
93
***
94
	declare CMlFrame Exp_Window <=> (Page.GetFirstChild("Window") as CMlFrame);
95
    Exp_Window.ZIndex = 10000.;
96
***
97
98
***FML_Loop***
99
***
100
101
102
        // handle pending events
103
        foreach (Event in PendingEvents) {
104
105
            // mouse hover states
106
            if (Event.Type == CMlScriptEvent::Type::MouseOver && Event.Control.HasClass("menuItem")) {
107
                onButtonOver(Event.Control);
108
            }
109
110
            if (Event.Type == CMlScriptEvent::Type::MouseOut && Event.Control.HasClass("menuItem")) {
111
                onButtonOut(Event.Control);
112
            }
113
        }
114
***
115
116
EOL
117
        );
118
119
120
        $button = $this->uiFactory->createButton("Admin", uiButton::TYPE_DECORATED);
121
        $button->setPosition(-150, 70);
122
        $button->setScale(1);
123
124
        $button->setAction($this->actionFactory->createManialinkAction($manialink, [$this, 'createMenu'],
125
            [$manialink]));
126
        $manialink->addChild($button);
127
        $manialink->addChild($this->currentMenuView);
128
    }
129
130
    public function createMenu($login, $input, $args)
131
    {
132
        $manialink = $args[0];
133
        $anim = $this->uiFactory->createAnimation();
134
135
136
        $frame = Frame::create();
137
138
        $menuButtons = $this->uiFactory->createLayoutRow(0, 20);
139
        $menuButtons->setAlign("center", "top");
140
141
142
        $basebtn = $this->uiFactory->createLabel("", uiLabel::TYPE_TITLE);
143
        $basebtn->addClass('menuItem')
144
            ->setOpacity(0)
145
            ->setSize(60, 8)
146
            ->setScriptEvents(true)
147
            ->setAlign("center", "center");
148
149
        $btn = clone $basebtn;
150
        $btn->setText("");
151
        $btn->setAction($this->actionFactory->createManialinkAction($manialink, [$this, "doHide"], [$manialink]));
152
        $menuButtons->addChild($btn);
153
154
        $btn = clone $basebtn;
155
        $btn->setText("Close menu");
156
        $btn->setAction($this->actionFactory->createManialinkAction($manialink, [$this, "doHide"], [$manialink]));
157
        $menuButtons->addChild($btn);
158
159
        $delay = 0;
160
        foreach ($menuButtons->getChildren() as $btn) {
161
            $anim->addAnimation($btn, "opacity='1'", 500, $delay, "Linear");
162
            $delay += 50;
163
        }
164
165
        $frame->addChild($menuButtons);
166
        $frame->addChild($this->createHeader("Server Menu"));
167
        $frame->addChild($anim);
168
169
        $this->currentMenuView = $frame;
170
        $group = $this->groupFactory->createForPlayer($login);
171
        $this->update($group);
172
173
    }
174
175
    protected function createHeader($title = "Server Menu")
176
    {
177
        $anim = $this->uiFactory->createAnimation();
178
179
        $headerFrame = Frame::create("background");
180
181
        $baseLabel = new Label();
182
        $baseLabel->setAreaColor("0000")
183
            ->setAreaFocusColor("0000")
184
            ->setTextColor("FFF");
185
        $baseLabel->setAlign("center", "center2")
186
            ->addClass("bg");
187
188
        $label = clone $baseLabel;
189
        $label->setText("")
190
            ->setTextSize(16)
191
            ->setPosition(0, 50)
192
            ->setSize(32, 32);
193
        $headerFrame->addChild($label);
194
195
        $label = clone $baseLabel;
196
        $label->setText($title)
197
            ->setTextSize(8)
198
            ->setPosition(0, 35)
199
            ->setTextFont('Oswald');
200
        $headerFrame->addChild($label);
201
202
203
        $quad = new Quad();
204
        $quad->addClass("bg")
205
            ->setPosition(0, 28)
206
            ->setSize(100, 0.5)
207
            ->setAlign("center", "center")
208
            ->setBackgroundColor("fff")
209
            ->setOpacity(0);
210
        $headerFrame->addChild($quad);
211
        $anim->addAnimation($quad, "opacity='1'", 300, 0, "Linear");
212
213
        $quad = new Quad();
214
        $quad->addClass("bg")
215
            ->setId("mainBg")
216
            ->setPosition(0, 0)
217
            ->setSize(322, 182);
218
        $quad->setAlign("center", "center")
219
            ->setStyles("Bgs1", "BgDialogBlur")
220
            ->setOpacity(0);
221
222
        $anim->addAnimation($quad, "opacity='1'", 300, 0, "Linear");
223
        $headerFrame->addChild($quad);
224
225
        return $headerFrame;
226
    }
227
228
    public function doHide($login, $input, $args)
229
    {
230
        $manialink = $args[0];
0 ignored issues
show
Unused Code introduced by
$manialink is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
231
232
        $this->currentMenuView = Frame::create();
233
234
        $group = $this->groupFactory->createForPlayer($login);
235
        $this->update($group);
236
    }
237
238
239
}
240