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-28 10:34 UTC

CdbXMLImporter::documentWithCdbXML() F

↳ Parent: CdbXMLImporter

Complexity

Conditions	11
Paths	384

Size

Total Lines	101
Code Lines	58

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	101
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\CalendarFactory;
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;

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

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

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