Test Failed
Branch develop (6f7954)
by Andreas
05:13 queued 02:18
created

JSONCatalogProvider   A

Complexity

Total Complexity 10

Size/Duplication

Total Lines 99
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 4

Importance

Changes 1
Bugs 0 Features 0
Metric Value
wmc 10
c 1
b 0
f 0
lcom 1
cbo 4
dl 0
loc 99
rs 10

3 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 6 1
A getCatalog() 0 21 3
B parseJSON() 0 26 6
1
<?php
2
3
namespace Wambo\Catalog;
4
5
use League\Flysystem\FilesystemInterface;
6
use Wambo\Catalog\Error\CatalogException;
7
use Wambo\Catalog\Error\JSONException;
8
use Wambo\Catalog\Mapper\CatalogMapper;
9
use Wambo\Catalog\Model\Catalog;
0 ignored issues
show
Bug introduced by
This use statement conflicts with another class in this namespace, Wambo\Catalog\Catalog.

Let’s assume that you have a directory layout like this:

.
|-- OtherDir
|   |-- Bar.php
|   `-- Foo.php
`-- SomeDir
    `-- Foo.php

and let’s assume the following content of Bar.php:

// Bar.php
namespace OtherDir;

use SomeDir\Foo; // This now conflicts the class OtherDir\Foo

If both files OtherDir/Foo.php and SomeDir/Foo.php are loaded in the same runtime, you will see a PHP error such as the following:

PHP Fatal error:  Cannot use SomeDir\Foo as Foo because the name is already in use in OtherDir/Foo.php

However, as OtherDir/Foo.php does not necessarily have to be loaded and the error is only triggered if it is loaded before OtherDir/Bar.php, this problem might go unnoticed for a while. In order to prevent this error from surfacing, you must import the namespace with a different alias:

// Bar.php
namespace OtherDir;

use SomeDir\Foo as SomeDirFoo; // There is no conflict anymore.
Loading history...
10
11
/**
12
 * Class JSONCatalogProvider reads product and catalog data from an JSON file.
13
 */
14
class JSONCatalogProvider implements CatalogProviderInterface
15
{
16
    /**
17
     * @var FilesystemInterface $filesystem The filesystem this Catalog instance works on
18
     */
19
    private $filesystem;
20
21
    /**
22
     * @var string $catalogFilePath The path to the JSON file containing the catalog in the given $filesystem
23
     */
24
    private $catalogFilePath;
25
26
    /**
27
     * @var CatalogMapper A catalog mapper for converting unstructured catalog data into Catalog models
28
     */
29
    private $catalogMapper;
30
31
    /**
32
     * Creates a new instance of the JSONCatalogProvider class.
33
     *
34
     * @param FilesystemInterface $filesystem      The filesystem this Catalog instance works on
35
     * @param string              $catalogFilePath The path to the JSON file containing the catalog in the given
36
     *                                             $filesystem
37
     *
38
     * @param CatalogMapper       $catalogMapper   A catalog mapper for converting unstructured catalog data into
39
     *                                             Catalog models
40
     */
41
    public function __construct(FilesystemInterface $filesystem, string $catalogFilePath, CatalogMapper $catalogMapper)
42
    {
43
        $this->filesystem = $filesystem;
44
        $this->catalogFilePath = $catalogFilePath;
45
        $this->catalogMapper = $catalogMapper;
46
    }
47
48
    /**
49
     * Get the product catalog.
50
     *
51
     * @return Catalog
52
     *
53
     * @throws CatalogException If the catalog could not be created.
54
     */
55
    public function getCatalog()
56
    {
57
        if ($this->filesystem->has($this->catalogFilePath) == false) {
0 ignored issues
show
Coding Style Best Practice introduced by
It seems like you are loosely comparing two booleans. Considering using the strict comparison === instead.

When comparing two booleans, it is generally considered safer to use the strict comparison operator.

Loading history...
58
            return array();
0 ignored issues
show
Bug Best Practice introduced by
The return type of return array(); (array) is incompatible with the return type declared by the interface Wambo\Catalog\CatalogProviderInterface::getCatalog of type Wambo\Catalog\Model\Catalog.

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...
59
        }
60
61
        try {
62
63
            $json = $this->filesystem->read($this->catalogFilePath);
64
            $catalogData = $this->parseJSON($json);
0 ignored issues
show
Security Bug introduced by
It seems like $json defined by $this->filesystem->read($this->catalogFilePath) on line 63 can also be of type false; however, Wambo\Catalog\JSONCatalogProvider::parseJSON() does only seem to accept string, did you maybe forget to handle an error condition?

This check looks for type mismatches where the missing type is false. This is usually indicative of an error condtion.

Consider the follow example

<?php

function getDate($date)
{
    if ($date !== null) {
        return new DateTime($date);
    }

    return false;
}

This function either returns a new DateTime object or false, if there was an error. This is a typical pattern in PHP programming to show that an error has occurred without raising an exception. The calling code should check for this returned false before passing on the value to another function or method that may not be able to handle a false.

Loading history...
65
66
            // convert the catalog data into a Catalog model
67
            $catalog = $this->catalogMapper->getCatalog($catalogData);
68
69
            return $catalog;
70
71
        } catch (\Exception $catalogException) {
72
            throw new CatalogException(sprintf("Unable to read catalog from %s: ", $this->catalogFilePath,
73
                $catalogException->getMessage()), $catalogException);
74
        }
75
    }
76
77
    /**
78
     * Parse the given JSON and convert it into an array.
79
     *
80
     * @param string $json JSON code
81
     *
82
     * @return array
83
     *
84
     * @throws JSONException If the given JSON could not be parsed
85
     */
86
    private function parseJSON($json)
87
    {
88
        $catalogData = json_decode($json, true);
89
90
        // handle errors
91
        switch (json_last_error()) {
92
93
            case JSON_ERROR_DEPTH:
94
                throw new JSONException("The maximum stack depth has been exceeded");
95
96
            case JSON_ERROR_STATE_MISMATCH:
97
                throw new JSONException("Invalid or malformed JSON");
98
99
            case JSON_ERROR_CTRL_CHAR:
100
                throw new JSONException("Control character error, possibly incorrectly encoded");
101
102
            case JSON_ERROR_SYNTAX:
103
                throw new JSONException("Syntax error");
104
105
            case JSON_ERROR_UTF8:
106
                throw new JSONException("Malformed UTF-8 characters, possibly incorrectly encoded");
107
108
        }
109
110
        return $catalogData;
111
    }
112
}