CdbXMLImporter::documentWithCdbXML() - Code Metrics - Inspection of "III-1635 Importing calendar from CdbXml." - cultuurnet/udb3-php - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#243)

by Luc

created 2016-11-30 10:56 UTC

CdbXMLImporter::documentWithCdbXML() F

↳ Parent: CdbXMLImporter

Complexity

Conditions	11
Paths	384

Size

Total Lines	103
Code Lines	58

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	103
rs	3.8181
c	0
b	0
f	0
cc	11
eloc	58
nc	384
nop	2

How to fix Long Method Complexity

<?php
/**
 * @file
 */

namespace CultuurNet\UDB3\Place\ReadModel\JSONLD;

use CultureFeed_Cdb_Data_File;
use CultuurNet\UDB3\CalendarFactoryInterface;
use CultuurNet\UDB3\LabelImporter;
use CultuurNet\UDB3\Offer\ReadModel\JSONLD\CdbXMLItemBaseImporter;

/**
 * Takes care of importing actors in the CdbXML format (UDB2) that represent
 * a place, into a UDB3 JSON-LD document.
 */
class CdbXMLImporter
{
    /**
     * @var CdbXMLItemBaseImporter
     */
    private $cdbXMLItemBaseImporter;

    /**
     * @var CalendarFactoryInterface
     */
    private $calendarFactory;

    /**
     * @param CdbXMLItemBaseImporter $dbXMLItemBaseImporter
     * @param CalendarFactoryInterface $calendarFactory
     */
    public function __construct(
        CdbXMLItemBaseImporter $dbXMLItemBaseImporter,
        CalendarFactoryInterface $calendarFactory
    ) {
        $this->cdbXMLItemBaseImporter = $dbXMLItemBaseImporter;
        $this->calendarFactory = $calendarFactory;
    }

    /**
     * Imports a UDB2 organizer actor into a UDB3 JSON-LD document.
     *
     * @param \stdClass                   $base
     *   The JSON-LD document object to start from.
     * @param \CultureFeed_Cdb_Item_Base|\CultureFeed_Cdb_Item_Actor $item
     *   The event/actor data from UDB2 to import.
     *
     * @return \stdClass
     *   A new JSON-LD document object with the UDB2 actor data merged in.
     */
    public function documentWithCdbXML(
        $base,
        \CultureFeed_Cdb_Item_Base $item
    ) {
        $jsonLD = clone $base;

        $detail = null;

        /** @var \CultureFeed_Cdb_Data_ActorDetail[] $details */
        $details = $item->getDetails();

        foreach ($details as $languageDetail) {
            // The first language detail found will be used to retrieve
            // properties from which in UDB3 are not any longer considered
            // to be language specific.
            if (!$detail) {
                $detail = $languageDetail;
            }
        }

        // make sure the description is an object as well before trying to add
        // translations
        if (empty($jsonLD->description)) {
            $jsonLD->description = new \stdClass();
        }

        $descriptions = [
            trim($detail->getShortDescription()),
            trim($detail->getLongDescription())
        ];
        $descriptions = array_filter($descriptions);
        if (count($descriptions) > 0) {
            $jsonLD->description->nl = implode('<br/>', $descriptions);
        }

        // make sure the name is an object as well before trying to add
        // translations
        if (empty($jsonLD->name)) {
            $jsonLD->name = new \stdClass();
        }
        $jsonLD->name->nl = $detail->getTitle();

        $this->cdbXMLItemBaseImporter->importPublicationInfo($item, $jsonLD);
        $this->cdbXMLItemBaseImporter->importAvailable($item, $jsonLD);
        $this->cdbXMLItemBaseImporter->importExternalId($item, $jsonLD);
        $this->cdbXMLItemBaseImporter->importWorkflowStatus($item, $jsonLD);

        // Address
        $contact_cdb = $item->getContactInfo();
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.
    }
}
        if ($contact_cdb) {
            /** @var \CultureFeed_Cdb_Data_Address[] $addresses */
            $addresses = $contact_cdb->getAddresses();

            foreach ($addresses as $address) {
                $address = $address->getPhysicalAddress();

                if ($address) {
                    $jsonLD->address = array(
                        'addressCountry' => $address->getCountry(),
                        'addressLocality' => $address->getCity(),
                        'postalCode' => $address->getZip(),
                        'streetAddress' =>
                            $address->getStreet() . ' ' .
                            $address->getHouseNumber(),
                    );

                    break;
                }
            }
        }

        // Booking info.
        $bookingInfo = array(
            'description' => '',
            'name' => 'standard price',
            'price' => 0.0,
            'priceCurrency' => 'EUR',
        );
        $price = $detail->getPrice();

        if ($price) {
            $bookingInfo['description'] = floatval($price->getDescription());
            $bookingInfo['name'] = floatval($price->getTitle());
            $bookingInfo['price'] = floatval($price->getValue());
        }
        $jsonLD->bookingInfo = $bookingInfo;

        $this->importPicture($detail, $jsonLD);
/** @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);
    }
}

        $labelImporter = new LabelImporter();
        $labelImporter->importLabels($item, $jsonLD);

        $this->importTerms($item, $jsonLD);

        if ($item instanceof \CultureFeed_Cdb_Item_Actor) {
            $calendar = $this->calendarFactory->createFromWeekScheme(
                $item->getWeekScheme()
            );
            $jsonLD = (object)array_merge((array)$jsonLD, $calendar->toJsonLd());
        }

        return $jsonLD;
    }

    public function eventDocumentWithCdbXML(
        $base,
        \CultureFeed_Cdb_Item_Base $item
    ) {
        $jsonLD = $this->documentWithCdbXML($base, $item);

        return $jsonLD;
    }

    /**
     * @param \CultureFeed_Cdb_Item_Base $actor
     * @param \stdClass $jsonLD
     */
    private function importTerms(\CultureFeed_Cdb_Item_Base $actor, $jsonLD)
    {
        $themeBlacklist = [];
        $categories = array();
        foreach ($actor->getCategories() as $category) {

            /* @var \Culturefeed_Cdb_Data_Category $category */
            if ($category && !in_array($category->getName(), $themeBlacklist)) {
                $categories[] = array(
                    'label' => $category->getName(),
                    'domain' => $category->getType(),
                    'id' => $category->getId(),
                );
            }
        }
        $jsonLD->terms = $categories;
    }

    /**
     * @param \CultureFeed_Cdb_Data_ActorDetail $detail
     * @param \stdClass $jsonLD
     *
     * This is based on code found in the culturefeed theme.
     * @see https://github.com/cultuurnet/culturefeed/blob/master/culturefeed_agenda/theme/theme.inc#L266-L284
     */
    private function importPicture($detail, $jsonLD)

    {
        $mainPicture = null;

        // first check if there is a media file that is main and has the PHOTO media type
        $photos = $detail->getMedia()->byMediaType(CultureFeed_Cdb_Data_File::MEDIA_TYPE_PHOTO);
        foreach ($photos as $photo) {
            if ($photo->isMain()) {
                $mainPicture = $photo;
            }
        }

        // the IMAGEWEB media type is deprecated but can still be used as a main image if there is no PHOTO
        if (empty($mainPicture)) {
            $images = $detail->getMedia()->byMediaType(CultureFeed_Cdb_Data_File::MEDIA_TYPE_IMAGEWEB);
            foreach ($images as $image) {
                if ($image->isMain()) {
                    $mainPicture = $image;
                }
            }
        }

        // if there is no explicit main image we just use the oldest picture of any type
        if (empty($mainPicture)) {
            $pictures = $detail->getMedia()->byMediaTypes(
                [
                    CultureFeed_Cdb_Data_File::MEDIA_TYPE_PHOTO,
                    CultureFeed_Cdb_Data_File::MEDIA_TYPE_IMAGEWEB
                ]
            );

            $pictures->rewind();
            $mainPicture = count($pictures) > 0 ? $pictures->current() : null;
        }

        if ($mainPicture) {
            $jsonLD->image = $mainPicture->getHLink();
        }
    }
}


1		<?php
2		/**
3		* @file
4		*/
5
6		namespace CultuurNet\UDB3\Place\ReadModel\JSONLD;
7
8		use CultureFeed_Cdb_Data_File;
9		use CultuurNet\UDB3\CalendarFactoryInterface;
10		use CultuurNet\UDB3\LabelImporter;
11		use CultuurNet\UDB3\Offer\ReadModel\JSONLD\CdbXMLItemBaseImporter;
12
13		/**
14		* Takes care of importing actors in the CdbXML format (UDB2) that represent
15		* a place, into a UDB3 JSON-LD document.
16		*/
17		class CdbXMLImporter
18		{
19		/**
20		* @var CdbXMLItemBaseImporter
21		*/
22		private $cdbXMLItemBaseImporter;
23
24		/**
25		* @var CalendarFactoryInterface
26		*/
27		private $calendarFactory;
28
29		/**
30		* @param CdbXMLItemBaseImporter $dbXMLItemBaseImporter
31		* @param CalendarFactoryInterface $calendarFactory
32		*/
33		public function __construct(
34		CdbXMLItemBaseImporter $dbXMLItemBaseImporter,
35		CalendarFactoryInterface $calendarFactory
36		) {
37		$this->cdbXMLItemBaseImporter = $dbXMLItemBaseImporter;
38		$this->calendarFactory = $calendarFactory;
39		}
40
41		/**
42		* Imports a UDB2 organizer actor into a UDB3 JSON-LD document.
43		*
44		* @param \stdClass $base
45		* The JSON-LD document object to start from.
46		* @param \CultureFeed_Cdb_Item_Base\|\CultureFeed_Cdb_Item_Actor $item
47		* The event/actor data from UDB2 to import.
48		*
49		* @return \stdClass
50		* A new JSON-LD document object with the UDB2 actor data merged in.
51		*/
52		public function documentWithCdbXML(
53		$base,
54		\CultureFeed_Cdb_Item_Base $item
55		) {
56		$jsonLD = clone $base;
57
58		$detail = null;
59
60		/** @var \CultureFeed_Cdb_Data_ActorDetail[] $details */
61		$details = $item->getDetails();
62
63		foreach ($details as $languageDetail) {
64		// The first language detail found will be used to retrieve
65		// properties from which in UDB3 are not any longer considered
66		// to be language specific.
67		if (!$detail) {
68		$detail = $languageDetail;
69		}
70		}
71
72		// make sure the description is an object as well before trying to add
73		// translations
74		if (empty($jsonLD->description)) {
75		$jsonLD->description = new \stdClass();
76		}
77
78		$descriptions = [
79		trim($detail->getShortDescription()),
80		trim($detail->getLongDescription())
81		];
82		$descriptions = array_filter($descriptions);
83		if (count($descriptions) > 0) {
84		$jsonLD->description->nl = implode('<br/>', $descriptions);
85		}
86
87		// make sure the name is an object as well before trying to add
88		// translations
89		if (empty($jsonLD->name)) {
90		$jsonLD->name = new \stdClass();
91		}
92		$jsonLD->name->nl = $detail->getTitle();
93
94		$this->cdbXMLItemBaseImporter->importPublicationInfo($item, $jsonLD);
95		$this->cdbXMLItemBaseImporter->importAvailable($item, $jsonLD);
96		$this->cdbXMLItemBaseImporter->importExternalId($item, $jsonLD);
97		$this->cdbXMLItemBaseImporter->importWorkflowStatus($item, $jsonLD);
98
99		// Address
100		$contact_cdb = $item->getContactInfo();
		0 ignored issues – show Bug introduced 2016-01-12 10:15 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `CultureFeed_Cdb_Item_Base` as the method `getContactInfo()` does only exist in the following sub-classes of `CultureFeed_Cdb_Item_Base`: `CultureFeed_Cdb_Item_Actor`, `CultureFeed_Cdb_Item_Event`. 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 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 parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
101		if ($contact_cdb) {
102		/** @var \CultureFeed_Cdb_Data_Address[] $addresses */
103		$addresses = $contact_cdb->getAddresses();
104
105		foreach ($addresses as $address) {
106		$address = $address->getPhysicalAddress();
107
108		if ($address) {
109		$jsonLD->address = array(
110		'addressCountry' => $address->getCountry(),
111		'addressLocality' => $address->getCity(),
112		'postalCode' => $address->getZip(),
113		'streetAddress' =>
114		$address->getStreet() . ' ' .
115		$address->getHouseNumber(),
116		);
117
118		break;
119		}
120		}
121		}
122
123		// Booking info.
124		$bookingInfo = array(
125		'description' => '',
126		'name' => 'standard price',
127		'price' => 0.0,
128		'priceCurrency' => 'EUR',
129		);
130		$price = $detail->getPrice();
131
132		if ($price) {
133		$bookingInfo['description'] = floatval($price->getDescription());
134		$bookingInfo['name'] = floatval($price->getTitle());
135		$bookingInfo['price'] = floatval($price->getValue());
136		}
137		$jsonLD->bookingInfo = $bookingInfo;
138
139		$this->importPicture($detail, $jsonLD);
		0 ignored issues – show Bug introduced 2016-10-21 17:27 UTC by Report Bug Copy Issue Report It seems like `$detail` defined by `null` on line `58` can be `null`; however, `CultuurNet\UDB3\Place\Re...porter::importPicture()` 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...
140
141		$labelImporter = new LabelImporter();
142		$labelImporter->importLabels($item, $jsonLD);
143
144		$this->importTerms($item, $jsonLD);
145
146		if ($item instanceof \CultureFeed_Cdb_Item_Actor) {
147		$calendar = $this->calendarFactory->createFromWeekScheme(
148		$item->getWeekScheme()
149		);
150		$jsonLD = (object)array_merge((array)$jsonLD, $calendar->toJsonLd());
151		}
152
153		return $jsonLD;
154		}
155
156		public function eventDocumentWithCdbXML(
157		$base,
158		\CultureFeed_Cdb_Item_Base $item
159		) {
160		$jsonLD = $this->documentWithCdbXML($base, $item);
161
162		return $jsonLD;
163		}
164
165		/**
166		* @param \CultureFeed_Cdb_Item_Base $actor
167		* @param \stdClass $jsonLD
168		*/
169		private function importTerms(\CultureFeed_Cdb_Item_Base $actor, $jsonLD)
170		{
171		$themeBlacklist = [];
172		$categories = array();
173	View Code Duplication	foreach ($actor->getCategories() as $category) {
		0 ignored issues – show Duplication introduced 2015-11-24 14:43 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...
174		/* @var \Culturefeed_Cdb_Data_Category $category */
175		if ($category && !in_array($category->getName(), $themeBlacklist)) {
176		$categories[] = array(
177		'label' => $category->getName(),
178		'domain' => $category->getType(),
179		'id' => $category->getId(),
180		);
181		}
182		}
183		$jsonLD->terms = $categories;
184		}
185
186		/**
187		* @param \CultureFeed_Cdb_Data_ActorDetail $detail
188		* @param \stdClass $jsonLD
189		*
190		* This is based on code found in the culturefeed theme.
191		* @see https://github.com/cultuurnet/culturefeed/blob/master/culturefeed_agenda/theme/theme.inc#L266-L284
192		*/
193	View Code Duplication	private function importPicture($detail, $jsonLD)
		0 ignored issues – show Duplication introduced 2016-10-21 17:27 UTC by Report Bug Copy Issue Report 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...
194		{
195		$mainPicture = null;
196
197		// first check if there is a media file that is main and has the PHOTO media type
198		$photos = $detail->getMedia()->byMediaType(CultureFeed_Cdb_Data_File::MEDIA_TYPE_PHOTO);
199		foreach ($photos as $photo) {
200		if ($photo->isMain()) {
201		$mainPicture = $photo;
202		}
203		}
204
205		// the IMAGEWEB media type is deprecated but can still be used as a main image if there is no PHOTO
206		if (empty($mainPicture)) {
207		$images = $detail->getMedia()->byMediaType(CultureFeed_Cdb_Data_File::MEDIA_TYPE_IMAGEWEB);
208		foreach ($images as $image) {
209		if ($image->isMain()) {
210		$mainPicture = $image;
211		}
212		}
213		}
214
215		// if there is no explicit main image we just use the oldest picture of any type
216		if (empty($mainPicture)) {
217		$pictures = $detail->getMedia()->byMediaTypes(
218		[
219		CultureFeed_Cdb_Data_File::MEDIA_TYPE_PHOTO,
220		CultureFeed_Cdb_Data_File::MEDIA_TYPE_IMAGEWEB
221		]
222		);
223
224		$pictures->rewind();
225		$mainPicture = count($pictures) > 0 ? $pictures->current() : null;
226		}
227
228		if ($mainPicture) {
229		$jsonLD->image = $mainPicture->getHLink();
230		}
231		}
232		}
233

cultuurnet / udb3-php

Pull Request — master (#243)

CdbXMLImporter::documentWithCdbXML() F

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Duplication Side-by-Side

Filter issues like