ImportConnector::getOrCreateVCardProperty()   C
last analyzed

Complexity

Conditions 19
Paths 43

Size

Total Lines 55
Code Lines 34

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 0
CRAP Score 380

Importance

Changes 1
Bugs 0 Features 0
Metric Value
c 1
b 0
f 0
dl 0
loc 55
ccs 0
cts 48
cp 0
rs 6.6276
cc 19
eloc 34
nc 43
nop 2
crap 380

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
/**
3
 * ownCloud - CSV Import connector
4
 *
5
 * @author Nicolas Mora
6
 * @copyright 2013-2014 Nicolas Mora [email protected]
7
 *
8
 * This library is free software; you can redistribute it and/or
9
 * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
10
 * License as published by the Free Software Foundation
11
 * version 3 of the License
12
 *
13
 * This library is distributed in the hope that it will be useful,
14
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
15
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the
16
 * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
17
 *
18
 * You should have received a copy of the GNU Affero General Public
19
 * License along with this library.	If not, see <http://www.gnu.org/licenses/>.
20
 *
21
 */
22
 
0 ignored issues
show
Coding Style introduced by
Tabs must be used to indent lines; spaces are not allowed
Loading history...
23
namespace OCA\Contacts\Connector;
24
25
use Sabre\VObject\StringUtil;
26
27
/**
28
 * Abstract class used to implement import classes
29
 */
30
abstract class ImportConnector {
31
32
	// XML Configuration, class SimpleXml format
33
	protected $configContent;
34
	
35
	/**
36
	 * @param \SimpleXMLElement $xml_config
37
	 */
38
	public function __construct($xml_config = null) {
0 ignored issues
show
Coding Style Naming introduced by
The parameter $xml_config is not named in camelCase.

This check marks parameter names that have not been written in camelCase.

In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes databaseConnectionString.

Loading history...
Coding Style Naming introduced by
The variable $xml_config is not named in camelCase.

This check marks variable names that have not been written in camelCase.

In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes databaseConnectionString.

Loading history...
39
		if ($xml_config != null) {
40
			$this->setConfig($xml_config);
41
		}
42
	}
43
	
44
	// returns a table containing converted elements from the input file
45
	abstract function getElementsFromInput($input, $limit=-1);
0 ignored issues
show
Best Practice introduced by
It is generally recommended to explicitly declare the visibility for methods.

Adding explicit visibility (private, protected, or public) is generally recommend to communicate to other developers how, and from where this method is intended to be used.

Loading history...
Comprehensibility Best Practice introduced by
It is recommend to declare an explicit visibility for getElementsFromInput.

Generally, we recommend to declare visibility for all methods in your source code. This has the advantage of clearly communication to other developers, and also yourself, how this method should be consumed.

If you are not sure which visibility to choose, it is a good idea to start with the most restrictive visibility, and then raise visibility as needed, i.e. start with private, and only raise it to protected if a sub-class needs to have access, or public if an external class needs access.

Loading history...
46
	
47
	// returns a single converted element
48
	abstract function convertElementToVCard($element);
0 ignored issues
show
Best Practice introduced by
It is generally recommended to explicitly declare the visibility for methods.

Adding explicit visibility (private, protected, or public) is generally recommend to communicate to other developers how, and from where this method is intended to be used.

Loading history...
Comprehensibility Best Practice introduced by
It is recommend to declare an explicit visibility for convertElementToVCard.

Generally, we recommend to declare visibility for all methods in your source code. This has the advantage of clearly communication to other developers, and also yourself, how this method should be consumed.

If you are not sure which visibility to choose, it is a good idea to start with the most restrictive visibility, and then raise visibility as needed, i.e. start with private, and only raise it to protected if a sub-class needs to have access, or public if an external class needs access.

Loading history...
49
	
50
	// returns the probability that the file matchs the current format
51
	abstract function getFormatMatch($file);
0 ignored issues
show
Best Practice introduced by
It is generally recommended to explicitly declare the visibility for methods.

Adding explicit visibility (private, protected, or public) is generally recommend to communicate to other developers how, and from where this method is intended to be used.

Loading history...
Comprehensibility Best Practice introduced by
It is recommend to declare an explicit visibility for getFormatMatch.

Generally, we recommend to declare visibility for all methods in your source code. This has the advantage of clearly communication to other developers, and also yourself, how this method should be consumed.

If you are not sure which visibility to choose, it is a good idea to start with the most restrictive visibility, and then raise visibility as needed, i.e. start with private, and only raise it to protected if a sub-class needs to have access, or public if an external class needs access.

Loading history...
52
	
53
	public function setConfig($xml_config) {
0 ignored issues
show
Coding Style Naming introduced by
The parameter $xml_config is not named in camelCase.

This check marks parameter names that have not been written in camelCase.

In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes databaseConnectionString.

Loading history...
Coding Style Naming introduced by
The variable $xml_config is not named in camelCase.

This check marks variable names that have not been written in camelCase.

In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes databaseConnectionString.

Loading history...
54
		$this->configContent = $xml_config;
55
	}
56
	
57
	/**
58
	 * @brief updates a property given in parameter with the value and using the importEntry to set the different parameters
59
	 * @param $property the property to update
60
	 * @param $importEntry the entry configuration to update in SimpleXml format
61
	 * @value the value to update
62
	 */
63
	protected function updateProperty(&$property, $importEntry, $value) {
64
		if (isset($property) && isset($importEntry) && isset($value)) {
65
			if (isset($importEntry->vcard_entry)) {
66
				if (isset($importEntry->vcard_entry['type'])) {
67
					$property->add('TYPE', StringUtil::convertToUTF8($importEntry->vcard_entry['type']));
68
				}
69
				if (isset($importEntry->vcard_entry->additional_property)) {
70
					foreach ($importEntry->vcard_entry->additional_property as $additionalProperty) {
71
						$property->add($additionalProperty['name'], $additionalProperty['value']);
72
					}
73
				}
74
				if (isset($importEntry->vcard_entry['prefix'])) {
75
					$value = $importEntry->vcard_entry['prefix'].$value;
76
				}
77
				if (isset($importEntry->vcard_entry['group'])) {
78
					$property->group = $importEntry->vcard_entry['group'];
79
				}
80
				if (isset($importEntry->vcard_entry['position'])) {
81
					$separator=";";
0 ignored issues
show
Unused Code introduced by
$separator is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
82
					if (isset($importEntry->vcard_entry['separator'])) {
83
						$separator=$importEntry->vcard_entry['separator'];
0 ignored issues
show
Unused Code introduced by
$separator is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
84
					}
85
					$position = $importEntry->vcard_entry['position'];
86
					$vArray = $property->getParts();
87
					$vArray[intval($position)] = StringUtil::convertToUTF8($value);
88
					$property->setParts($vArray);
89
				} else {
90
					if (isset($importEntry->vcard_entry['value'])) {
91
						$property->add('TYPE', StringUtil::convertToUTF8($value));
92
					} else {
93
						$curVal = $property->getParts();
94
						$curVal[] = StringUtil::convertToUTF8($value);
95
						$property->setValue($curVal);
96
					}
97
				}
98
			}
99
			if (isset($importEntry->vcard_parameter)) {
100
				$property->add($importEntry->vcard_parameter['parameter'], StringUtil::convertToUTF8($value));
101
			}
102
		}
103
	}
104
		
105
	/**
106
	 * @brief modifies a vcard property array with the image
107
	 */
108
	public function updateImageProperty(&$property, $entry, $version=null) {
109
		$image = new \OC_Image();
110
		$image->loadFromData($entry);
111 View Code Duplication
		if (strcmp($version, '4.0') == 0) {
0 ignored issues
show
Duplication introduced by
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...
112
			$type = $image->mimeType();
113
		} else {
114
			$arrayType = explode('/', $image->mimeType());
115
			$type = strtoupper(array_pop($arrayType));
116
		}
117
		$property->add('ENCODING', 'b');
118
		$property->add('TYPE', $type);
119
		$property->setValue($image->__toString());
120
	}
121
122
	/**
123
	 * @brief returns the vcard property corresponding to the parameter
124
	 * creates the property if it doesn't exists yet
125
	 * @param $vcard the vcard to get or create the properties with
126
	 * @param $importEntry the parameter to find
127
	 * @return the property|false
128
	 */
129
	protected function getOrCreateVCardProperty(&$vcard, $importEntry) {
0 ignored issues
show
Coding Style Naming introduced by
The variable $additional_property is not named in camelCase.

This check marks variable names that have not been written in camelCase.

In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes databaseConnectionString.

Loading history...
130
		
131
		if (isset($vcard) && isset($importEntry)) {
132
			// looking for a property with the same name
133
			$properties = $vcard->select($importEntry['property']);
134
			foreach ($properties as $property) {
135
				if ($importEntry['type'] == null && !isset($importEntry->additional_property)) {
136
					return $property;
137
				}
138
				foreach ($property->parameters as $parameter) {
139
					// Filtering types
140
					if ($parameter->name == 'TYPE' && !strcmp($parameter->getValue(), $importEntry['type'])) {
141
						$found=0;
142
						if (isset($importEntry->additional_property)) {
143
							// Filtering additional properties if necessary (I know, there are a lot of inner loops, sorry)
144
							foreach($importEntry->additional_property as $additional_property) {
145
								if ((string)$parameter->name == $additional_property['name']) {
146
									$found++;
147
								}
148
							}
149
							if ($found == count($importEntry->additional_property)) {
150
								return $property;
151
							}
152
						}
153
						return $property;
154
					}
155
				}
156
				
157
				if (isset($importEntry['group']) && $property->group == $importEntry['group']) {
158
					return $property;
159
				}
160
			}		
161
			
162
			// Property not found, creating one
163
			$property = $vcard->createProperty($importEntry['property']);
164
			$vcard->add($property);
165
			if ($importEntry['type']!=null) {
166
				$property->add('TYPE', StringUtil::convertToUTF8($importEntry['type']));
167
			}
168
			switch ($importEntry['property']) {
169
				case "ADR":
170
					$property->setValue(array('', '', '', '', '', '', ''));
171
					break;
172
				case "N":
173
					$property->setValue(array('', '', '', '', ''));
174
					break;
175
			}
176
			if ($importEntry['group']!=null) {
177
				$property->group = $importEntry['group'];
178
			}
179
			return $property;
180
		} else {
181
			return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type documented by OCA\Contacts\Connector\I...etOrCreateVCardProperty of type OCA\Contacts\Connector\the.

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...
182
		}
183
	}
184
}
185
186
?>
0 ignored issues
show
Best Practice introduced by
It is not recommended to use PHP's closing tag ?> in files other than templates.

Using a closing tag in PHP files that only contain PHP code is not recommended as you might accidentally add whitespace after the closing tag which would then be output by PHP. This can cause severe problems, for example headers cannot be sent anymore.

A simple precaution is to leave off the closing tag as it is not required, and it also has no negative effects whatsoever.

Loading history...
187