Completed
Push — master ( 53beba...902569 )
by Barney
03:03
created

ObjectStorageTrait::append()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 4
Code Lines 2

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 4
c 0
b 0
f 0
rs 10
cc 1
eloc 2
nc 1
nop 2
1
<?php
2
3
namespace Shrikeh\Collection;
4
5
use Traversable;
6
use OuterIterator;
7
use SplObjectStorage;
8
9
use Shrikeh\Collection\Exception\IncorrectInterface;
10
11
trait ObjectStorageTrait
12
{
13
    public function __construct(Traversable $objects)
14
    {
15
        if (!$this instanceof OuterIterator) {
16
            $msg = 'class %s uses trait %s but it is not an %s';
17
            throw new IncorrectInterface(
18
                sprintf($msg,
19
                    __CLASS__,
20
                    __TRAIT__,
21
                    'OuterIterator'
22
                )
23
            );
24
        }
25
        parent::__construct(new SplObjectStorage());
26
27
        foreach ($objects as $key => $value) {
28
            $this->append($value, $key);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface OuterIterator as the method append() does only exist in the following implementations of said interface: AppendIterator, Shrikeh\Collection\ImmutableBoilerplate.

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...
29
        }
30
        $this->getStorage()->rewind();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface OuterIterator as the method getStorage() does only exist in the following implementations of said interface: Shrikeh\Collection\ImmutableBoilerplate.

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...
31
    }
32
33
    abstract protected function getStorage();
34
35
    protected function append($data, $key)
0 ignored issues
show
Unused Code introduced by
The parameter $data 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 $key 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...
36
    {
37
        throw new \LogicException('you must override the append() method');
38
    }
39
}
40