Holding - Code Metrics - Inspection of "Adding options to save holdings and items" - scriptotek/php-alma-client - Measure and Improve Code Quality continuously with Scrutinizer

Test Setup Failed

Pull Request — master (#15)

unknown

created 2020-12-04 10:44 UTC

Holding A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	106
Duplicated Lines	8.49 %

Coupling/Cohesion

Components	2
Dependencies	6

Importance

Changes

Metric	Value
wmc	10
lcom	2
cbo	6
dl	9
loc	106
rs	10
c	0
b	0
f	0

9 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	7	1
A	fetchData()	0	4	1
A	isInitialized()	0	4	2
A	setMarcRecord()	0	6	1
A	save()	9	9	1
A	onData()	0	4	1
A	urlBase()	0	4	1
A	getRecord()	0	4	1
A	getItems()	0	4	1

How to fix Duplicated Code

<?php

namespace Scriptotek\Alma\Bibs;

use Danmichaelo\QuiteSimpleXMLElement\QuiteSimpleXMLElement;
use Scriptotek\Alma\Client;
use Scriptotek\Alma\Model\LazyResource;
use Scriptotek\Marc\Record as MarcRecord;

/**
 * A single Holding resource.
 */
class Holding extends LazyResource
{
    /* @var string */
    public $holding_id;

    /* @var Bib */
    public $bib;

    /* @var Items */
    public $items;

    /* @var MarcRecord */
    protected $_marc;

    public function __construct(Client $client, Bib $bib, $holding_id)
    {
        parent::__construct($client);
        $this->bib = $bib;
        $this->holding_id = $holding_id;
        $this->items = Items::make($this->client, $bib, $this);
    }

    /**
     * Get the model data.
     */
    protected function fetchData()
    {
        return $this->client->getXML($this->url());
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());
}
    }

    /**
     * Check if we have the full representation of our data object.
     *
     * @param $data
     *
     * @return bool
     */
    protected function isInitialized($data)
    {
        return is_a($data, QuiteSimpleXMLElement::class) && $data->has('record');
    }

    /**
     * Update the MARC record on this holding object. Chainable method.
     *
     * @param string $xml
     *
     * @return Holding
     */
    public function setMarcRecord($xml)
    {
        $this->_marc = MarcRecord::fromString($xml);

        return $this;
    }


    /**
     * Save the holding.
     */
    public function save()

    {
        $marcXml = $this->getRecord()->toXML('UTF-8', false, false);
        $marcXml = new QuiteSimpleXMLElement($marcXml);
        $this->data->first('record')->replace($marcXml);
        $newData = $this->data->asXML();

        return $this->client->putXML($this->url(), $newData);
    }

    /**
     * Called when data is available to be processed.
     *
     * @param mixed $data
     */
    protected function onData($data)
    {
        $this->setMarcRecord($data->first('record')->asXML());
    }

    /**
     * Generate the base URL for this resource.
     *
     * @return string
     */
    protected function urlBase()
    {
        return "/bibs/{$this->bib->mms_id}/holdings/{$this->holding_id}";
    }

    /**
     * Get the MARC record for this holding object.
     */
    public function getRecord()
    {
        return $this->init()->_marc;
    }

    /**
     * Get the items for this holding.
     */
    public function getItems()
    {
        return $this->items;
    }
}


1		<?php
2
3		namespace Scriptotek\Alma\Bibs;
4
5		use Danmichaelo\QuiteSimpleXMLElement\QuiteSimpleXMLElement;
6		use Scriptotek\Alma\Client;
7		use Scriptotek\Alma\Model\LazyResource;
8		use Scriptotek\Marc\Record as MarcRecord;
9
10		/**
11		* A single Holding resource.
12		*/
13		class Holding extends LazyResource
14		{
15		/* @var string */
16		public $holding_id;
17
18		/* @var Bib */
19		public $bib;
20
21		/* @var Items */
22		public $items;
23
24		/* @var MarcRecord */
25		protected $_marc;
26
27		public function __construct(Client $client, Bib $bib, $holding_id)
28		{
29		parent::__construct($client);
30		$this->bib = $bib;
31		$this->holding_id = $holding_id;
32		$this->items = Items::make($this->client, $bib, $this);
33		}
34
35		/**
36		* Get the model data.
37		*/
38		protected function fetchData()
39		{
40		return $this->client->getXML($this->url());
		0 ignored issues – show Bug Best Practice introduced 2018-11-09 19:22 UTC by Report Bug Copy Issue Report The return type of `return $this->client->getXML($this->url());` (`Danmichaelo\QuiteSimpleX...t\QuiteSimpleXMLElement`) is incompatible with the return type of the parent method `Scriptotek\Alma\Model\LazyResource::fetchData` of type `stdClass`. 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...
41		}
42
43		/**
44		* Check if we have the full representation of our data object.
45		*
46		* @param $data
47		*
48		* @return bool
49		*/
50		protected function isInitialized($data)
51		{
52		return is_a($data, QuiteSimpleXMLElement::class) && $data->has('record');
53		}
54
55		/**
56		* Update the MARC record on this holding object. Chainable method.
57		*
58		* @param string $xml
59		*
60		* @return Holding
61		*/
62		public function setMarcRecord($xml)
63		{
64		$this->_marc = MarcRecord::fromString($xml);
65
66		return $this;
67		}
68
69
70		/**
71		* Save the holding.
72		*/
73	View Code Duplication	public function save()
		0 ignored issues – show Duplication introduced 2019-11-04 15:47 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...
74		{
75		$marcXml = $this->getRecord()->toXML('UTF-8', false, false);
76		$marcXml = new QuiteSimpleXMLElement($marcXml);
77		$this->data->first('record')->replace($marcXml);
78		$newData = $this->data->asXML();
79
80		return $this->client->putXML($this->url(), $newData);
81		}
82
83		/**
84		* Called when data is available to be processed.
85		*
86		* @param mixed $data
87		*/
88		protected function onData($data)
89		{
90		$this->setMarcRecord($data->first('record')->asXML());
91		}
92
93		/**
94		* Generate the base URL for this resource.
95		*
96		* @return string
97		*/
98		protected function urlBase()
99		{
100		return "/bibs/{$this->bib->mms_id}/holdings/{$this->holding_id}";
101		}
102
103		/**
104		* Get the MARC record for this holding object.
105		*/
106		public function getRecord()
107		{
108		return $this->init()->_marc;
109		}
110
111		/**
112		* Get the items for this holding.
113		*/
114		public function getItems()
115		{
116		return $this->items;
117		}
118		}
119

scriptotek / php-alma-client

Pull Request — master (#15)

Holding A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

9 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like