FileUploadObserver - Code Metrics - Inspection of "Refactor file upload functionality" - techdivision/import-product - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#92)

by Tim

created 2017-09-02 10:40 UTC

FileUploadObserver A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	104
Duplicated Lines	0 %

Coupling/Cohesion

Components	1
Dependencies	4

Test Coverage

Coverage

Importance

Changes

Metric	Value
wmc	10
c	0
b	0
f	0
lcom	1
cbo	4
dl	0
loc	104
ccs	0
cts	56
cp	0
rs	10

2 Methods

Rating	Name	Duplication	Size	Complexity
C	process()	0	85	9
A	getImageTypes()	0	4	1

<?php

/**
 * TechDivision\Import\Product\Media\Observers\FileUploadObserver
 *
 * NOTICE OF LICENSE
 *
 * This source file is subject to the Open Software License (OSL 3.0)
 * that is available through the world-wide-web at this URL:
 * http://opensource.org/licenses/osl-3.0.php
 *
 * PHP version 5
 *
 * @author    Tim Wagner <[email protected]>
 * @copyright 2016 TechDivision GmbH <[email protected]>
 * @license   http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
 * @link      https://github.com/techdivision/import-product
 * @link      http://www.techdivision.com
 */

namespace TechDivision\Import\Product\Observers;

use TechDivision\Import\Product\Utils\ColumnKeys;
use TechDivision\Import\Product\Utils\ConfigurationKeys;

/**
 * Observer that uploads the product image files specified in a CSV file to a
 * configurable directory.
 *
 * @author    Tim Wagner <[email protected]>
 * @copyright 2016 TechDivision GmbH <[email protected]>
 * @license   http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
 * @link      https://github.com/techdivision/import-product
 * @link      http://www.techdivision.com
 */
class FileUploadObserver extends AbstractProductImportObserver
{

    /**
     * Process the observer's business logic.
     *
     * @return array The processed row
     */
    protected function process()
    {

        // query whether or not we've the flag to upload the image files has been set
        if ($this->getSubject()->getConfiguration()->hasParam(ConfigurationKeys::COPY_IMAGES)) {
            // query whether or not we've to upload the image files
            if ($this->getSubject()->getConfiguration()->getParam(ConfigurationKeys::COPY_IMAGES)) {
                // initialize the array for the actual images
                $actualImageNames = array();

                // iterate over the available image fields
                foreach (array_keys($this->getImageTypes()) as $imageColumnName) {
                    // do nothing if the column is empty
                    if (!$this->hasValue($imageColumnName)) {
                        continue;
                    }

                    // override the image path with the new one, if the image has already been processed
                    if (isset($actualImageNames[$imageName = $this->getValue($imageColumnName)])) {
                        $this->setValue($imageColumnName, $actualImageNames[$imageName]);
                        continue;
                    }

                    // upload the file and set the new image path
                    $imagePath = $this->getSubject()->uploadFile($imageName);
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.
    }
}

                    // log a message that the image has been copied
                    $this->getSubject()
                         ->getSystemLogger()
                         ->debug(
                             sprintf(
                                 'Successfully copied image type %s with name %s => %s',
                                 $imageColumnName,
                                 $imageName,
                                 $imagePath
                             )
                         );

                    // override the image path with the new one
                    $this->setValue($imageColumnName, $imagePath);

                        // add the image to the list with processed images
                    $actualImageNames[$imageName] = $imagePath;
                }

                // query whether or not, we've additional images
                if ($additionalImages = $this->getValue(ColumnKeys::ADDITIONAL_IMAGES, null, array($this, 'explode'))) {
                    // process all the additional images
                    foreach ($additionalImages as $key => $additionalImageName) {
                        // query whether or not the image has already been processed
                        if (isset($actualImageNames[$additionalImageName])) {
                            // if yes, override the image path
                            $additionalImages[$key] = $imagePath;
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}
                            // and override the image path with the new one
                            $this->setValue($imageColumnName, $actualImageNames[$additionalImageName]);
foreach ($a as $b) {
}

// $b is defined here only if $a has elements, for example if $a is array()
// then $b would not be defined here. To avoid that, we recommend to set a
// default value for $b.


// Better
$b = 0; // or whatever default makes sense in your context
foreach ($a as $b) {
}

// $b is now guaranteed to be defined here.
                            continue;
                        }

                        // upload the file and set the new image path
                        $imagePath = $this->getSubject()->uploadFile($additionalImageName);
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.
    }
}

                        // log a message that the image has been copied
                        $this->getSubject()
                             ->getSystemLogger()
                             ->debug(
                                 sprintf(
                                     'Successfully copied additional image wth name %s => %s',
                                     $additionalImageName,
                                     $imagePath
                                 )
                             );

                        // override the image path
                        $additionalImages[$key] = $imagePath;

                        // add the image to the list with processed images
                        $actualImageNames[$additionalImageName] = $imagePath;
                    }

                    // override the image paths with the new one
                    $this->setValue(ColumnKeys::ADDITIONAL_IMAGES, implode(',', $additionalImages));
                }
            }
        }
    }

    /**
     * Return's the array with the available image types and their label columns.
     *
     * @return array The array with the available image types
     */
    protected function getImageTypes()
    {
        return $this->getSubject()->getImageTypes();
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.
    }
}
    }
}


1			<?php
2
3			/**
4			* TechDivision\Import\Product\Media\Observers\FileUploadObserver
5			*
6			* NOTICE OF LICENSE
7			*
8			* This source file is subject to the Open Software License (OSL 3.0)
9			* that is available through the world-wide-web at this URL:
10			* http://opensource.org/licenses/osl-3.0.php
11			*
12			* PHP version 5
13			*
14			* @author Tim Wagner <[email protected]>
15			* @copyright 2016 TechDivision GmbH <[email protected]>
16			* @license http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
17			* @link https://github.com/techdivision/import-product
18			* @link http://www.techdivision.com
19			*/
20
21			namespace TechDivision\Import\Product\Observers;
22
23			use TechDivision\Import\Product\Utils\ColumnKeys;
24			use TechDivision\Import\Product\Utils\ConfigurationKeys;
25
26			/**
27			* Observer that uploads the product image files specified in a CSV file to a
28			* configurable directory.
29			*
30			* @author Tim Wagner <[email protected]>
31			* @copyright 2016 TechDivision GmbH <[email protected]>
32			* @license http://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
33			* @link https://github.com/techdivision/import-product
34			* @link http://www.techdivision.com
35			*/
36			class FileUploadObserver extends AbstractProductImportObserver
37			{
38
39			/**
40			* Process the observer's business logic.
41			*
42			* @return array The processed row
43			*/
44			protected function process()
45			{
46
47			// query whether or not we've the flag to upload the image files has been set
48			if ($this->getSubject()->getConfiguration()->hasParam(ConfigurationKeys::COPY_IMAGES)) {
49			// query whether or not we've to upload the image files
50			if ($this->getSubject()->getConfiguration()->getParam(ConfigurationKeys::COPY_IMAGES)) {
51			// initialize the array for the actual images
52			$actualImageNames = array();
53
54			// iterate over the available image fields
55			foreach (array_keys($this->getImageTypes()) as $imageColumnName) {
56			// do nothing if the column is empty
57			if (!$this->hasValue($imageColumnName)) {
58			continue;
59			}
60
61			// override the image path with the new one, if the image has already been processed
62			if (isset($actualImageNames[$imageName = $this->getValue($imageColumnName)])) {
63			$this->setValue($imageColumnName, $actualImageNames[$imageName]);
64			continue;
65			}
66
67			// upload the file and set the new image path
68			$imagePath = $this->getSubject()->uploadFile($imageName);
			0 ignored issues – show Bug introduced 2017-09-02 10:43 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `TechDivision\Import\Subjects\SubjectInterface` as the method `uploadFile()` does only exist in the following implementations of said interface: `TechDivision\Import\Obse...s\FileUploadSubjectImpl`, `TechDivision\Import\Product\Subjects\BunchSubject`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
69
70			// log a message that the image has been copied
71			$this->getSubject()
72			->getSystemLogger()
73			->debug(
74			sprintf(
75			'Successfully copied image type %s with name %s => %s',
76			$imageColumnName,
77			$imageName,
78			$imagePath
79			)
80			);
81
82			// override the image path with the new one
83			$this->setValue($imageColumnName, $imagePath);
84
85			// add the image to the list with processed images
86			$actualImageNames[$imageName] = $imagePath;
87			}
88
89			// query whether or not, we've additional images
90			if ($additionalImages = $this->getValue(ColumnKeys::ADDITIONAL_IMAGES, null, array($this, 'explode'))) {
91			// process all the additional images
92			foreach ($additionalImages as $key => $additionalImageName) {
93			// query whether or not the image has already been processed
94			if (isset($actualImageNames[$additionalImageName])) {
95			// if yes, override the image path
96			$additionalImages[$key] = $imagePath;
			0 ignored issues – show Bug introduced 2017-09-02 10:43 UTC by Report Bug Copy Issue Report The variable `$imagePath` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
97			// and override the image path with the new one
98			$this->setValue($imageColumnName, $actualImageNames[$additionalImageName]);
			0 ignored issues – show Bug introduced 2017-09-02 10:43 UTC by Report Bug Copy Issue Report The variable `$imageColumnName` seems to be defined by a `foreach` iteration on line `55`. Are you sure the iterator is never empty, otherwise this variable is not defined? It seems like you are relying on a variable being defined by an iteration: foreach ($a as $b) { } // $b is defined here only if $a has elements, for example if $a is array() // then $b would not be defined here. To avoid that, we recommend to set a // default value for $b. // Better $b = 0; // or whatever default makes sense in your context foreach ($a as $b) { } // $b is now guaranteed to be defined here. Loading history...
99			continue;
100			}
101
102			// upload the file and set the new image path
103			$imagePath = $this->getSubject()->uploadFile($additionalImageName);
			0 ignored issues – show Bug introduced 2017-09-02 10:43 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `TechDivision\Import\Subjects\SubjectInterface` as the method `uploadFile()` does only exist in the following implementations of said interface: `TechDivision\Import\Obse...s\FileUploadSubjectImpl`, `TechDivision\Import\Product\Subjects\BunchSubject`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
104
105			// log a message that the image has been copied
106			$this->getSubject()
107			->getSystemLogger()
108			->debug(
109			sprintf(
110			'Successfully copied additional image wth name %s => %s',
111			$additionalImageName,
112			$imagePath
113			)
114			);
115
116			// override the image path
117			$additionalImages[$key] = $imagePath;
118
119			// add the image to the list with processed images
120			$actualImageNames[$additionalImageName] = $imagePath;
121			}
122
123			// override the image paths with the new one
124			$this->setValue(ColumnKeys::ADDITIONAL_IMAGES, implode(',', $additionalImages));
125			}
126			}
127			}
128			}
129
130			/**
131			* Return's the array with the available image types and their label columns.
132			*
133			* @return array The array with the available image types
134			*/
135			protected function getImageTypes()
136			{
137			return $this->getSubject()->getImageTypes();
			0 ignored issues – show Bug introduced 2017-09-02 10:43 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `TechDivision\Import\Subjects\SubjectInterface` as the method `getImageTypes()` does only exist in the following implementations of said interface: `TechDivision\Import\Prod...\AbstractProductSubject`, `TechDivision\Import\Product\Subjects\BunchSubject`. 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
138			}
139			}
140

techdivision / import-product

Pull Request — master (#92)

FileUploadObserver A

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

2 Methods

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like