YmlMetadataDriver - Code Metrics - Inspection of "[WIP] Release 2.0" - bankiru/doctrine-api-client - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#14)

by Pavel

created 2016-10-21 07:05 UTC

YmlMetadataDriver B

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	187
Duplicated Lines	13.9 %

Coupling/Cohesion

Components	1
Dependencies	6

Importance

Changes

Metric	Value
wmc	41
lcom	1
cbo	6
dl	26
loc	187
rs	8.2769
c	0
b	0
f	0

4 Methods

Rating	Name	Duplication	Size	Complexity
F	loadMetadataForClass()	0	87	24
C	mapAssociation()	26	43	12
A	loadMappingFile()	0	4	1
B	fieldToArray()	0	22	4

How to fix Duplicated Code Complexity

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:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like YmlMetadataDriver often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use YmlMetadataDriver, and based on these observations, apply Extract Interface, too.

<?php

namespace Bankiru\Api\Doctrine\Mapping\Driver;

use Bankiru\Api\Doctrine\Exception\MappingException;
use Bankiru\Api\Doctrine\Mapping\ApiMetadata;
use Bankiru\Api\Doctrine\Mapping\EntityMetadata;
use Bankiru\Api\Doctrine\Rpc\Method\EntityMethodProvider;
use Bankiru\Api\Doctrine\Rpc\Method\MethodProvider;
use Doctrine\Common\Persistence\Mapping\ClassMetadata;
use Doctrine\Common\Persistence\Mapping\Driver\FileDriver;
use Symfony\Component\Yaml\Exception\ParseException;
use Symfony\Component\Yaml\Yaml;

class YmlMetadataDriver extends FileDriver
{
    /**
     * Loads the metadata for the specified class into the provided container.
     *
     * @param string                       $className
     * @param EntityMetadata|ClassMetadata $metadata
     *
     * @return void
     * @throws MappingException
     */
    public function loadMetadataForClass($className, ClassMetadata $metadata)
    {
        $element = $this->getElement($className);

        switch ($element['type']) {
            case 'entity':
                if (array_key_exists('repositoryClass', $element)) {
                    $metadata->setCustomRepositoryClass($element['repositoryClass']);
                }
                break;
            case 'mappedSuperclass':
                $metadata->isMappedSuperclass = true;
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
                $metadata->setCustomRepositoryClass(
                    array_key_exists('repositoryClass', $element) ? $element['repositoryClass'] : null
                );
                break;
        }

        // Configure API
        if (array_key_exists('api', $element)) {
            if (array_key_exists('name', $element['api'])) {
                $metadata->apiName = $element['api']['name'];
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
            }
        }

        // Configure Client
        if (array_key_exists('client', $element)) {
            if (array_key_exists('name', $element['client'])) {
                $metadata->clientName = $element['client']['name'];
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
            }

            $methodProvider = null;
            if (array_key_exists('methods', $element['client'])) {
                $methodProvider = new MethodProvider($element['client']['methods']);
            }
            if (array_key_exists('entityPath', $element['client'])) {
                $pathSeparator  =
                    array_key_exists('entityPathSeparator', $element['client']) ?
                        $element['client']['entityPathSeparator'] :
                        EntityMethodProvider::DEFAULT_PATH_SEPARATOR;
                $methodProvider =
                    new EntityMethodProvider($element['client']['entityPath'], $pathSeparator, $methodProvider);
            }

            if (null === $methodProvider && null === $metadata->methodProvider) {
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
                throw MappingException::noMethods();
            }

            if (null !== $methodProvider) {
                $metadata->methodProvider = $methodProvider;
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
            }
        }

        // Configure fields
        if (array_key_exists('fields', $element)) {
            foreach ($element['fields'] as $field => $mapping) {
                $mapping = $this->fieldToArray($field, $mapping);
                $metadata->mapField($mapping);
            }
        }

        // Configure identifiers
        $associationIds = [];
        if (array_key_exists('id', $element)) {
            // Evaluate identifier settings
            foreach ($element['id'] as $name => $idElement) {
                if (isset($idElement['associationKey']) && (bool)$idElement['associationKey'] === true) {
                    $associationIds[$name] = true;
                    continue;
                }

                $mapping = $this->fieldToArray($name, $idElement);

                $mapping['id'] = true;
                $metadata->mapField($mapping);
            }
        }

        foreach (['oneToOne', 'manyToOne', 'oneToMany'] as $type) {
            if (array_key_exists($type, $element)) {
                $associations = $element[$type];
                foreach ($associations as $name => $association) {
                    $this->mapAssociation($metadata, $type, $name, $association, $associationIds);

                }
            }
        }
    }

    /**
     * @param EntityMetadata $metadata
     * @param string         $type
     * @param string         $name
     * @param array          $association
     * @param int[]          $associationIds
     */
    protected function mapAssociation(EntityMetadata $metadata, $type, $name, $association, $associationIds)
    {
        $mapping           = $this->fieldToArray($name, $association);
        $mapping['target'] = $association['target'];
        if (isset($association['fetch'])) {
            $mapping['fetch'] = constant(ApiMetadata::class . '::FETCH_' . $association['fetch']);
        }
        switch ($type) {
            case 'oneToOne':

                $mapping['type'] = EntityMetadata::ONE_TO_ONE;
                if (isset($associationIds[$mapping['field']])) {
                    $mapping['id'] = true;
                }
                if (array_key_exists('mappedBy', $association)) {
                    $mapping['mappedBy'] = $association['mappedBy'];
                }
                if (array_key_exists('inversedBy', $association)) {
                    $mapping['inversedBy'] = $association['inversedBy'];
                }
                $metadata->mapOneToOne($mapping);
                break;
            case 'manyToOne':
                $mapping['type'] = EntityMetadata::MANY_TO_ONE;
                if (array_key_exists('inversedBy', $association)) {
                    $mapping['inversedBy'] = $association['inversedBy'];
                }
                $metadata->mapManyToOne($mapping);
                break;
            case 'oneToMany':

                $mapping['type'] = EntityMetadata::ONE_TO_MANY;
                if (array_key_exists('mappedBy', $association)) {
                    $mapping['mappedBy'] = $association['mappedBy'];
                }
                if (array_key_exists('orderBy', $association)) {
                    $mapping['orderBy'] = $association['orderBy'];
                }
                if (array_key_exists('indexBy', $association)) {
                    $mapping['indexBy'] = $association['indexBy'];
                }
                $metadata->mapOneToMany($mapping);
                break;
        }
    }

    /**
     * Loads a mapping file with the given name and returns a map
     * from class/entity names to their corresponding file driver elements.
     *
     * @param string $file The mapping file to load.
     *
     * @return array
     * @throws ParseException
     */
    protected function loadMappingFile($file)
    {
        return Yaml::parse(file_get_contents($file));
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());
}
    }

    private function fieldToArray($field, $source)
    {
        $mapping = [
            'field'    => $field,
            'type'     => 'string',
            'nullable' => true,
        ];

        if (array_key_exists('type', $source)) {
            $mapping['type'] = $source['type'];
        }

        if (array_key_exists('nullable', $source)) {
            $mapping['nullable'] = $source['nullable'];
        }

        if (array_key_exists('api_field', $source)) {
            $mapping['api_field'] = $source['api_field'];
        }

        return $mapping;
    }
}



Pull Request — master (#14)

YmlMetadataDriver B

Complexity

Size/Duplication

Coupling/Cohesion

Importance

4 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Available Fixes

1		<?php
2
3		namespace Bankiru\Api\Doctrine\Mapping\Driver;
4
5		use Bankiru\Api\Doctrine\Exception\MappingException;
6		use Bankiru\Api\Doctrine\Mapping\ApiMetadata;
7		use Bankiru\Api\Doctrine\Mapping\EntityMetadata;
8		use Bankiru\Api\Doctrine\Rpc\Method\EntityMethodProvider;
9		use Bankiru\Api\Doctrine\Rpc\Method\MethodProvider;
10		use Doctrine\Common\Persistence\Mapping\ClassMetadata;
11		use Doctrine\Common\Persistence\Mapping\Driver\FileDriver;
12		use Symfony\Component\Yaml\Exception\ParseException;
13		use Symfony\Component\Yaml\Yaml;
14
15		class YmlMetadataDriver extends FileDriver
16		{
17		/**
18		* Loads the metadata for the specified class into the provided container.
19		*
20		* @param string $className
21		* @param EntityMetadata\|ClassMetadata $metadata
22		*
23		* @return void
24		* @throws MappingException
25		*/
26		public function loadMetadataForClass($className, ClassMetadata $metadata)
27		{
28		$element = $this->getElement($className);
29
30		switch ($element['type']) {
31		case 'entity':
32		if (array_key_exists('repositoryClass', $element)) {
33		$metadata->setCustomRepositoryClass($element['repositoryClass']);
34		}
35		break;
36		case 'mappedSuperclass':
37		$metadata->isMappedSuperclass = true;
		0 ignored issues – show Bug introduced 2016-05-12 09:51 UTC by Report Bug Copy Issue Report Accessing `isMappedSuperclass` on the interface `Doctrine\Common\Persistence\Mapping\ClassMetadata` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
38		$metadata->setCustomRepositoryClass(
39		array_key_exists('repositoryClass', $element) ? $element['repositoryClass'] : null
40		);
41		break;
42		}
43
44		// Configure API
45		if (array_key_exists('api', $element)) {
46		if (array_key_exists('name', $element['api'])) {
47		$metadata->apiName = $element['api']['name'];
		0 ignored issues – show Bug introduced 2016-10-20 19:34 UTC by Report Bug Copy Issue Report Accessing `apiName` on the interface `Doctrine\Common\Persistence\Mapping\ClassMetadata` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
48		}
49		}
50
51		// Configure Client
52		if (array_key_exists('client', $element)) {
53		if (array_key_exists('name', $element['client'])) {
54		$metadata->clientName = $element['client']['name'];
		0 ignored issues – show Bug introduced 2016-05-12 09:51 UTC by Report Bug Copy Issue Report Accessing `clientName` on the interface `Doctrine\Common\Persistence\Mapping\ClassMetadata` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
55		}
56
57		$methodProvider = null;
58		if (array_key_exists('methods', $element['client'])) {
59		$methodProvider = new MethodProvider($element['client']['methods']);
60		}
61		if (array_key_exists('entityPath', $element['client'])) {
62		$pathSeparator =
63		array_key_exists('entityPathSeparator', $element['client']) ?
64		$element['client']['entityPathSeparator'] :
65		EntityMethodProvider::DEFAULT_PATH_SEPARATOR;
66		$methodProvider =
67		new EntityMethodProvider($element['client']['entityPath'], $pathSeparator, $methodProvider);
68		}
69
70		if (null === $methodProvider && null === $metadata->methodProvider) {
		0 ignored issues – show Bug introduced 2016-10-21 07:28 UTC by Report Bug Copy Issue Report Accessing `methodProvider` on the interface `Doctrine\Common\Persistence\Mapping\ClassMetadata` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
71		throw MappingException::noMethods();
72		}
73
74		if (null !== $methodProvider) {
75		$metadata->methodProvider = $methodProvider;
		0 ignored issues – show Bug introduced 2016-05-12 09:51 UTC by Report Bug Copy Issue Report Accessing `methodProvider` on the interface `Doctrine\Common\Persistence\Mapping\ClassMetadata` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
76		}
77		}
78
79		// Configure fields
80		if (array_key_exists('fields', $element)) {
81		foreach ($element['fields'] as $field => $mapping) {
82		$mapping = $this->fieldToArray($field, $mapping);
83		$metadata->mapField($mapping);
84		}
85		}
86
87		// Configure identifiers
88		$associationIds = [];
89		if (array_key_exists('id', $element)) {
90		// Evaluate identifier settings
91		foreach ($element['id'] as $name => $idElement) {
92		if (isset($idElement['associationKey']) && (bool)$idElement['associationKey'] === true) {
93		$associationIds[$name] = true;
94		continue;
95		}
96
97		$mapping = $this->fieldToArray($name, $idElement);
98
99		$mapping['id'] = true;
100		$metadata->mapField($mapping);
101		}
102		}
103
104		foreach (['oneToOne', 'manyToOne', 'oneToMany'] as $type) {
105		if (array_key_exists($type, $element)) {
106		$associations = $element[$type];
107		foreach ($associations as $name => $association) {
108		$this->mapAssociation($metadata, $type, $name, $association, $associationIds);
		0 ignored issues – show Compatibility introduced 2016-05-12 09:51 UTC by Report Bug Copy Issue Report `$metadata` of type `object<Doctrine\Common\P...\Mapping\ClassMetadata>` is not a sub-type of `object<Bankiru\Api\Doctr...Mapping\EntityMetadata>`. It seems like you assume a concrete implementation of the interface `Doctrine\Common\Persistence\Mapping\ClassMetadata` 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...
109		}
110		}
111		}
112		}
113
114		/**
115		* @param EntityMetadata $metadata
116		* @param string $type
117		* @param string $name
118		* @param array $association
119		* @param int[] $associationIds
120		*/
121		protected function mapAssociation(EntityMetadata $metadata, $type, $name, $association, $associationIds)
122		{
123		$mapping = $this->fieldToArray($name, $association);
124		$mapping['target'] = $association['target'];
125		if (isset($association['fetch'])) {
126		$mapping['fetch'] = constant(ApiMetadata::class . '::FETCH_' . $association['fetch']);
127		}
128		switch ($type) {
129	View Code Duplication	case 'oneToOne':
		0 ignored issues – show Duplication introduced 2016-05-12 09:51 UTC by Report Bug Copy Issue Report This code seems to be duplicated across 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...
130		$mapping['type'] = EntityMetadata::ONE_TO_ONE;
131		if (isset($associationIds[$mapping['field']])) {
132		$mapping['id'] = true;
133		}
134		if (array_key_exists('mappedBy', $association)) {
135		$mapping['mappedBy'] = $association['mappedBy'];
136		}
137		if (array_key_exists('inversedBy', $association)) {
138		$mapping['inversedBy'] = $association['inversedBy'];
139		}
140		$metadata->mapOneToOne($mapping);
141		break;
142		case 'manyToOne':
143		$mapping['type'] = EntityMetadata::MANY_TO_ONE;
144		if (array_key_exists('inversedBy', $association)) {
145		$mapping['inversedBy'] = $association['inversedBy'];
146		}
147		$metadata->mapManyToOne($mapping);
148		break;
149	View Code Duplication	case 'oneToMany':
		0 ignored issues – show Duplication introduced 2016-05-12 09:51 UTC by Report Bug Copy Issue Report This code seems to be duplicated across 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...
150		$mapping['type'] = EntityMetadata::ONE_TO_MANY;
151		if (array_key_exists('mappedBy', $association)) {
152		$mapping['mappedBy'] = $association['mappedBy'];
153		}
154		if (array_key_exists('orderBy', $association)) {
155		$mapping['orderBy'] = $association['orderBy'];
156		}
157		if (array_key_exists('indexBy', $association)) {
158		$mapping['indexBy'] = $association['indexBy'];
159		}
160		$metadata->mapOneToMany($mapping);
161		break;
162		}
163		}
164
165		/**
166		* Loads a mapping file with the given name and returns a map
167		* from class/entity names to their corresponding file driver elements.
168		*
169		* @param string $file The mapping file to load.
170		*
171		* @return array
172		* @throws ParseException
173		*/
174		protected function loadMappingFile($file)
175		{
176		return Yaml::parse(file_get_contents($file));
		0 ignored issues – show Bug Best Practice introduced 2016-10-07 06:22 UTC by Report Bug Copy Issue Report The return type of `return \Symfony\Componen...e_get_contents($file));` (`string\|array\|stdClass`) is incompatible with the return type declared by the abstract method `Doctrine\Common\Persiste...Driver::loadMappingFile` of type `array`. 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...
177		}
178
179		private function fieldToArray($field, $source)
180		{
181		$mapping = [
182		'field' => $field,
183		'type' => 'string',
184		'nullable' => true,
185		];
186
187		if (array_key_exists('type', $source)) {
188		$mapping['type'] = $source['type'];
189		}
190
191		if (array_key_exists('nullable', $source)) {
192		$mapping['nullable'] = $source['nullable'];
193		}
194
195		if (array_key_exists('api_field', $source)) {
196		$mapping['api_field'] = $source['api_field'];
197		}
198
199		return $mapping;
200		}
201		}
202
203

bankiru / doctrine-api-client

Pull Request — master (#14)

YmlMetadataDriver B

Complexity

Size/Duplication

Coupling/Cohesion

Importance

4 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like