Completed
Pull Request — master (#111)
by MusikAnimal
02:25
created

EditCounterController   A

Complexity

Total Complexity 35

Size/Duplication

Total Lines 399
Duplicated Lines 42.86 %

Coupling/Cohesion

Components 1
Dependencies 9

Importance

Changes 1
Bugs 0 Features 1
Metric Value
wmc 35
lcom 1
cbo 9
dl 171
loc 399
rs 9
c 1
b 0
f 1

15 Methods

Rating   Name   Duplication   Size   Complexity  
A getToolShortname() 0 4 1
B setUpEditCounter() 0 27 5
A indexAction() 0 19 3
B resultAction() 0 27 3
A generalStatsAction() 17 17 2
A namespaceTotalsAction() 17 17 2
A timecardAction() 21 21 2
A yearcountsAction() 17 17 2
A monthcountsAction() 21 21 2
A latestglobalAction() 18 18 3
A pairDataApiAction() 12 12 2
A logCountsApiAction() 12 12 2
A editSizesApiAction() 12 12 2
A namespaceTotalsApiAction() 12 12 2
A monthcountsApiAction() 12 12 2

How to fix   Duplicated Code   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

1
<?php
2
/**
3
 * This file contains only the EditCounterController class.
4
 */
5
6
namespace AppBundle\Controller;
7
8
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
9
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
10
use Symfony\Component\HttpFoundation\Request;
11
use Symfony\Component\HttpFoundation\Response;
12
use Symfony\Component\HttpFoundation\RedirectResponse;
13
use Symfony\Component\HttpFoundation\JsonResponse;
14
use Xtools\EditCounter;
15
use Xtools\EditCounterRepository;
16
use Xtools\Page;
17
use Xtools\Project;
18
use Xtools\ProjectRepository;
19
use Xtools\User;
20
use Xtools\UserRepository;
21
22
/**
23
 * Class EditCounterController
24
 */
25
class EditCounterController extends XtoolsController
26
{
27
28
    /** @var User The user being queried. */
29
    protected $user;
30
31
    /** @var Project The project being queried. */
32
    protected $project;
33
34
    /** @var EditCounter The edit-counter, that does all the work. */
35
    protected $editCounter;
36
37
    /**
38
     * Get the tool's shortname.
39
     * @return string
40
     * @codeCoverageIgnore
41
     */
42
    public function getToolShortname()
43
    {
44
        return 'ec';
45
    }
46
47
    /**
48
     * Every action in this controller (other than 'index') calls this first.
49
     * If a response is returned, the calling action is expected to return it.
50
     * @param Request $request
51
     * @param string $key API key, as given in the reuqest. Omit this for actions
52
     *   that are public (only /api/ec actions should pass this in).
53
     * @return null|RedirectResponse
54
     */
55
    protected function setUpEditCounter(Request $request, $key = null)
56
    {
57
        // Return the EditCounter if we already have one.
58
        if ($this->editCounter instanceof EditCounter) {
59
            return;
60
        }
61
62
        // Validate key if attempted to make internal API request.
63
        if ($key && (string)$key !== (string)$this->container->getParameter('secret')) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $key of type string|null is loosely compared to true; this is ambiguous if the string can be empty. You might want to explicitly use !== null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
64
            throw $this->createAccessDeniedException('This endpoint is for internal use only.');
65
        }
66
67
        // Will redirect to Simple Edit Counter if they have too many edits.
68
        $ret = $this->validateProjectAndUser($request, 'SimpleEditCounterResult');
69
        if ($ret instanceof RedirectResponse) {
70
            return $ret;
71
        } else {
72
            // Get Project and User instances.
73
            list($this->project, $this->user) = $ret;
74
        }
75
76
        // Instantiate EditCounter.
77
        $editCounterRepo = new EditCounterRepository();
78
        $editCounterRepo->setContainer($this->container);
1 ignored issue
show
Compatibility introduced by
$this->container of type object<Symfony\Component...ion\ContainerInterface> is not a sub-type of object<Symfony\Component...ncyInjection\Container>. It seems like you assume a concrete implementation of the interface Symfony\Component\Depend...tion\ContainerInterface 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...
79
        $this->editCounter = new EditCounter($this->project, $this->user);
0 ignored issues
show
Bug introduced by
It seems like $this->user can be null; however, __construct() 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...
80
        $this->editCounter->setRepository($editCounterRepo);
81
    }
82
83
    /**
84
     * The initial GET request that displays the search form.
85
     *
86
     * @Route("/ec", name="ec")
87
     * @Route("/ec", name="EditCounter")
88
     * @Route("/ec/", name="EditCounterSlash")
89
     * @Route("/ec/index.php", name="EditCounterIndexPhp")
90
     * @Route("/ec/{project}", name="EditCounterProject")
91
     *
92
     * @param Request $request
93
     * @return RedirectResponse|Response
94
     */
95
    public function indexAction(Request $request)
96
    {
97
        $params = $this->parseQueryParams($request);
98
99
        if (isset($params['project']) && isset($params['username'])) {
100
            return $this->redirectToRoute('EditCounterResult', $params);
101
        }
102
103
        // Convert the given project (or default project) into a Project instance.
104
        $params['project'] = $this->getProjectFromQuery($params);
105
106
        // Otherwise fall through.
107
        return $this->render('editCounter/index.html.twig', [
108
            'xtPageTitle' => 'tool-ec',
109
            'xtSubtitle' => 'tool-ec-desc',
110
            'xtPage' => 'ec',
111
            'project' => $params['project'],
112
        ]);
113
    }
114
115
    /**
116
     * Display all results.
117
     * @Route("/ec/{project}/{username}", name="EditCounterResult")
118
     * @param Request $request
119
     * @param string $project
120
     * @param string $username
121
     * @return Response
122
     * @codeCoverageIgnore
123
     */
124
    public function resultAction(Request $request, $project, $username)
0 ignored issues
show
Unused Code introduced by
The parameter $project is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
Unused Code introduced by
The parameter $username is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
125
    {
126
        $ret = $this->setUpEditCounter($request);
127
        if ($ret instanceof RedirectResponse) {
128
            return $ret;
129
        }
130
131
        // Asynchronously collect some of the data that will be shown.
132
        // If multithreading is turned off, the normal getters in the views will
133
        // collect the necessary data synchronously.
134
        if ($this->container->getParameter('app.multithread')) {
135
            $this->editCounter->prepareData($this->container);
0 ignored issues
show
Unused Code introduced by
The call to EditCounter::prepareData() has too many arguments starting with $this->container.

This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.

If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.

In this case you can add the @ignore PhpDoc annotation to the duplicate definition and it will be ignored.

Loading history...
136
        }
137
138
        // FIXME: is this needed? It shouldn't ever be a subrequest here in the resultAction.
139
        $isSubRequest = $this->container->get('request_stack')->getParentRequest() !== null;
140
141
        return $this->render('editCounter/result.html.twig', [
142
            'xtTitle' => $this->user->getUsername() . ' - ' . $this->project->getTitle(),
143
            'xtPage' => 'ec',
144
            'base_dir' => realpath($this->getParameter('kernel.root_dir').'/..'),
145
            'is_sub_request' => $isSubRequest,
146
            'user' => $this->user,
147
            'project' => $this->project,
148
            'ec' => $this->editCounter,
149
        ]);
150
    }
151
152
    /**
153
     * Display the general statistics section.
154
     * @Route("/ec-generalstats/{project}/{username}", name="EditCounterGeneralStats")
155
     * @param Request $request
156
     * @return Response
157
     * @codeCoverageIgnore
158
     */
159 View Code Duplication
    public function generalStatsAction(Request $request)
0 ignored issues
show
Duplication introduced by
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...
160
    {
161
        $ret = $this->setUpEditCounter($request);
162
        if ($ret instanceof RedirectResponse) {
163
            return $ret;
164
        }
165
166
        $isSubRequest = $this->get('request_stack')->getParentRequest() !== null;
167
        return $this->render('editCounter/general_stats.html.twig', [
168
            'xtTitle' => $this->user->getUsername(),
169
            'xtPage' => 'ec',
170
            'is_sub_request' => $isSubRequest,
171
            'user' => $this->user,
172
            'project' => $this->project,
173
            'ec' => $this->editCounter,
174
        ]);
175
    }
176
177
    /**
178
     * Display the namespace totals section.
179
     * @Route("/ec-namespacetotals/{project}/{username}", name="EditCounterNamespaceTotals")
180
     * @param Request $request
181
     * @return Response
182
     * @codeCoverageIgnore
183
     */
184 View Code Duplication
    public function namespaceTotalsAction(Request $request)
0 ignored issues
show
Duplication introduced by
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...
185
    {
186
        $ret = $this->setUpEditCounter($request);
187
        if ($ret instanceof RedirectResponse) {
188
            return $ret;
189
        }
190
191
        $isSubRequest = $this->get('request_stack')->getParentRequest() !== null;
192
        return $this->render('editCounter/namespace_totals.html.twig', [
193
            'xtTitle' => $this->user->getUsername(),
194
            'xtPage' => 'ec',
195
            'is_sub_request' => $isSubRequest,
196
            'user' => $this->user,
197
            'project' => $this->project,
198
            'ec' => $this->editCounter,
199
        ]);
200
    }
201
202
    /**
203
     * Display the timecard section.
204
     * @Route("/ec-timecard/{project}/{username}", name="EditCounterTimecard")
205
     * @param Request $request
206
     * @return Response
207
     * @codeCoverageIgnore
208
     */
209 View Code Duplication
    public function timecardAction(Request $request)
0 ignored issues
show
Duplication introduced by
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...
210
    {
211
        $ret = $this->setUpEditCounter($request);
212
        if ($ret instanceof RedirectResponse) {
213
            return $ret;
214
        }
215
216
        $isSubRequest = $this->get('request_stack')->getParentRequest() !== null;
217
        $optedInPage = $this->project
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Xtools\Repository as the method getPage() does only exist in the following sub-classes of Xtools\Repository: Xtools\ProjectRepository. 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

  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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
218
            ->getRepository()
219
            ->getPage($this->project, $this->project->userOptInPage($this->user));
220
        return $this->render('editCounter/timecard.html.twig', [
221
            'xtTitle' => $this->user->getUsername(),
222
            'xtPage' => 'ec',
223
            'is_sub_request' => $isSubRequest,
224
            'user' => $this->user,
225
            'project' => $this->project,
226
            'ec' => $this->editCounter,
227
            'opted_in_page' => $optedInPage,
228
        ]);
229
    }
230
231
    /**
232
     * Display the year counts section.
233
     * @Route("/ec-yearcounts/{project}/{username}", name="EditCounterYearCounts")
234
     * @param Request $request
235
     * @return Response
236
     * @codeCoverageIgnore
237
     */
238 View Code Duplication
    public function yearcountsAction(Request $request)
0 ignored issues
show
Duplication introduced by
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...
239
    {
240
        $ret = $this->setUpEditCounter($request);
241
        if ($ret instanceof RedirectResponse) {
242
            return $ret;
243
        }
244
245
        $isSubRequest = $this->container->get('request_stack')->getParentRequest() !== null;
246
        return $this->render('editCounter/yearcounts.html.twig', [
247
            'xtTitle' => $this->user->getUsername(),
248
            'xtPage' => 'ec',
249
            'is_sub_request' => $isSubRequest,
250
            'user' => $this->user,
251
            'project' => $this->project,
252
            'ec' => $this->editCounter,
253
        ]);
254
    }
255
256
    /**
257
     * Display the month counts section.
258
     * @Route("/ec-monthcounts/{project}/{username}", name="EditCounterMonthCounts")
259
     * @param Request $request
260
     * @return Response
261
     * @codeCoverageIgnore
262
     */
263 View Code Duplication
    public function monthcountsAction(Request $request)
0 ignored issues
show
Duplication introduced by
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...
264
    {
265
        $ret = $this->setUpEditCounter($request);
266
        if ($ret instanceof RedirectResponse) {
267
            return $ret;
268
        }
269
270
        $isSubRequest = $this->container->get('request_stack')->getParentRequest() !== null;
271
        $optedInPage = $this->project
1 ignored issue
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Xtools\Repository as the method getPage() does only exist in the following sub-classes of Xtools\Repository: Xtools\ProjectRepository. 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

  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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
272
            ->getRepository()
273
            ->getPage($this->project, $this->project->userOptInPage($this->user));
274
        return $this->render('editCounter/monthcounts.html.twig', [
275
            'xtTitle' => $this->user->getUsername(),
276
            'xtPage' => 'ec',
277
            'is_sub_request' => $isSubRequest,
278
            'user' => $this->user,
279
            'project' => $this->project,
280
            'ec' => $this->editCounter,
281
            'opted_in_page' => $optedInPage,
282
        ]);
283
    }
284
285
    /**
286
     * Display the latest global edits section.
287
     * @Route("/ec-latestglobal/{project}/{username}", name="EditCounterLatestGlobal")
288
     * @param Request $request The HTTP request.
289
     * @return Response
290
     * @codeCoverageIgnore
291
     */
292 View Code Duplication
    public function latestglobalAction(Request $request)
0 ignored issues
show
Duplication introduced by
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...
293
    {
294
        $ret = $this->setUpEditCounter($request);
295
        if ($ret instanceof RedirectResponse) {
296
            return $ret;
297
        }
298
299
        $isSubRequest = $request->get('htmlonly')
300
                        || $this->container->get('request_stack')->getParentRequest() !== null;
301
        return $this->render('editCounter/latest_global.html.twig', [
302
            'xtTitle' => $this->user->getUsername(),
303
            'xtPage' => 'ec',
304
            'is_sub_request' => $isSubRequest,
305
            'user' => $this->user,
306
            'project' => $this->project,
307
            'ec' => $this->editCounter,
308
        ]);
309
    }
310
311
312
    /**
313
     * Below are internal API endpoints for the Edit Counter.
314
     * All only respond with JSON and only to requests passing in the value
315
     * of the 'secret' parameter. This should not be used in JavaScript or clientside
316
     * applications, rather only used internally.
317
     */
318
319
    /**
320
     * Get (most) of the general statistics as JSON.
321
     * @Route("/api/ec/pairdata/{project}/{username}/{key}", name="EditCounterApiPairData")
322
     * @param Request $request
323
     * @param string $key API key.
324
     * @return JsonResponse
325
     * @codeCoverageIgnore
326
     */
327 View Code Duplication
    public function pairDataApiAction(Request $request, $key)
0 ignored issues
show
Duplication introduced by
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...
328
    {
329
        $ret = $this->setUpEditCounter($request, $key);
330
        if ($ret instanceof RedirectResponse) {
331
            return $ret;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $ret; (Symfony\Component\HttpFoundation\RedirectResponse) is incompatible with the return type documented by AppBundle\Controller\Edi...ller::pairDataApiAction of type Symfony\Component\HttpFoundation\JsonResponse.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
332
        }
333
334
        return new JsonResponse(
335
            $this->editCounter->getPairData(),
336
            Response::HTTP_OK
337
        );
338
    }
339
340
    /**
341
     * Get various log counts for the user as JSON.
342
     * @Route("/api/ec/logcounts/{project}/{username}/{key}", name="EditCounterApiLogCounts")
343
     * @param Request $request
344
     * @param string $key API key.
345
     * @return JsonResponse
346
     * @codeCoverageIgnore
347
     */
348 View Code Duplication
    public function logCountsApiAction(Request $request, $key)
0 ignored issues
show
Duplication introduced by
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...
349
    {
350
        $ret = $this->setUpEditCounter($request, $key);
351
        if ($ret instanceof RedirectResponse) {
352
            return $ret;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $ret; (Symfony\Component\HttpFoundation\RedirectResponse) is incompatible with the return type documented by AppBundle\Controller\Edi...ler::logCountsApiAction of type Symfony\Component\HttpFoundation\JsonResponse.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
353
        }
354
355
        return new JsonResponse(
356
            $this->editCounter->getLogCounts(),
357
            Response::HTTP_OK
358
        );
359
    }
360
361
    /**
362
     * Get edit sizes for the user as JSON.
363
     * @Route("/api/ec/editsizes/{project}/{username}/{key}", name="EditCounterApiEditSizes")
364
     * @param Request $request
365
     * @param string $key API key.
366
     * @return JsonResponse
367
     * @codeCoverageIgnore
368
     */
369 View Code Duplication
    public function editSizesApiAction(Request $request, $key)
0 ignored issues
show
Duplication introduced by
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...
370
    {
371
        $ret = $this->setUpEditCounter($request, $key);
372
        if ($ret instanceof RedirectResponse) {
373
            return $ret;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $ret; (Symfony\Component\HttpFoundation\RedirectResponse) is incompatible with the return type documented by AppBundle\Controller\Edi...ler::editSizesApiAction of type Symfony\Component\HttpFoundation\JsonResponse.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
374
        }
375
376
        return new JsonResponse(
377
            $this->editCounter->getEditSizeData(),
378
            Response::HTTP_OK
379
        );
380
    }
381
382
    /**
383
     * Get the namespace totals for the user as JSON.
384
     * @Route("/api/ec/namespacetotals/{project}/{username}/{key}", name="EditCounterApiNamespaceTotals")
385
     * @param Request $request
386
     * @param string $key API key.
387
     * @return Response
388
     * @codeCoverageIgnore
389
     */
390 View Code Duplication
    public function namespaceTotalsApiAction(Request $request, $key)
0 ignored issues
show
Duplication introduced by
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...
391
    {
392
        $ret = $this->setUpEditCounter($request, $key);
393
        if ($ret instanceof RedirectResponse) {
394
            return $ret;
395
        }
396
397
        return new JsonResponse(
398
            $this->editCounter->namespaceTotals(),
399
            Response::HTTP_OK
400
        );
401
    }
402
403
    /**
404
     * Display or fetch the month counts for the user.
405
     * @Route("/api/ec/monthcounts/{project}/{username}/{key}", name="EditCounterApiMonthCounts")
406
     * @param Request $request
407
     * @param string $key API key.
408
     * @return Response
409
     * @codeCoverageIgnore
410
     */
411 View Code Duplication
    public function monthcountsApiAction(Request $request, $key)
0 ignored issues
show
Duplication introduced by
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...
412
    {
413
        $ret = $this->setUpEditCounter($request, $key);
414
        if ($ret instanceof RedirectResponse) {
415
            return $ret;
416
        }
417
418
        return new JsonResponse(
419
            $this->editCounter->monthCounts(),
420
            Response::HTTP_OK
421
        );
422
    }
423
}
424