Completed
Branch master (55a138)
by Michael
03:21
created

ExtcalPersistableObjectHandler   D

Complexity

Total Complexity 134

Size/Duplication

Total Lines 704
Duplicated Lines 23.58 %

Coupling/Cohesion

Components 1
Dependencies 0

Importance

Changes 0
Metric Value
dl 166
loc 704
rs 4.4444
c 0
b 0
f 0
wmc 134
lcom 1
cbo 0

19 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 10 2
A create() 0 9 2
A get() 0 18 4
B getObjects() 8 22 5
C convertResultSet() 18 34 7
D getList() 8 37 10
D getCount() 34 34 10
B deleteById() 5 23 5
D insert() 5 75 25
C updateAll() 5 25 7
B updateFieldValue() 5 17 5
B deleteAll() 0 15 5
A _toObject() 0 18 3
C objectToArray() 10 62 14
A objectToArrayWithoutExternalKey() 0 12 3
A updateCounter() 0 11 2
D getSum() 34 34 10
D getMax() 34 34 10
A getAvg() 0 16 4

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like ExtcalPersistableObjectHandler often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use ExtcalPersistableObjectHandler, and based on these observations, apply Extract Interface, too.

1
<?php namespace XoopsModules\Extcal;
2
/*
3
 * You may not change or alter any portion of this comment or credits
4
 * of supporting developers from this source code or any supporting source code
5
 * which is considered copyrighted (c) material of the original comment or credit authors.
6
 *
7
 * This program is distributed in the hope that it will be useful,
8
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
9
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
10
 */
11
12
/**
13
 * @copyright    {@link https://xoops.org/ XOOPS Project}
14
 * @license      {@link http://www.gnu.org/licenses/gpl-2.0.html GNU GPL 2 or later}
15
 * @package      extcal
16
 * @since
17
 * @author       XOOPS Development Team,
18
 */
19
20
use XoopsModules\Extcal;
21
22
/**
23
 * Persistable Object Handler class.
24
 * This class is responsible for providing data access mechanisms to the data source
25
 * of derived class objects.
26
 *
27
 * @author    Jan Keller Pedersen <[email protected]> - IDG Danmark A/S <www.idg.dk>
28
 * @copyright copyright (c) 2000-2004 XOOPS.org
29
 */
30
class ExtcalPersistableObjectHandler extends \XoopsPersistableObjectHandler //XoopsObjectHandler
31
{
32
    /**#@+
33
     * Information about the class, the handler is managing
34
     *
35
     * @var string
36
     */
37
    //    public $table;
38
    //    public $keyName;
39
    //    public $className;
40
    //    public $identifierName;
41
42
    /**#@-*/
43
44
    /**
45
     * Constructor - called from child classes.
46
     *
47
     * @param XoopsDatabase $db        {@link XoopsDatabase} object
0 ignored issues
show
Documentation introduced by
Should the type for parameter $db not be \XoopsDatabase?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
48
     * @param string        $tablename Name of database table
49
     * @param string        $classname Name of Class, this handler is managing
50
     * @param string        $keyname   Name of the property, holding the key
51
     * @param bool          $idenfierName
52
     */
53
    public function __construct(\XoopsDatabase $db, $tablename, $classname, $keyname, $idenfierName = false)
54
    {
55
        parent::__construct($db);
56
        $this->table     = $db->prefix($tablename);
57
        $this->keyName   = $keyname;
58
        $this->className = $classname;
59
        if (false !== $idenfierName) {
60
            $this->identifierName = $idenfierName;
61
        }
62
    }
63
64
    /**
65
     * Constructor.
66
     */
67
    //    public function ExtcalPersistableObjectHandler($db, $tablename, $classname, $keyname, $idenfierName = false)
0 ignored issues
show
Unused Code Comprehensibility introduced by
57% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
68
    //    {
69
    //        $this->__construct($db, $tablename, $classname, $keyname, $idenfierName);
0 ignored issues
show
Unused Code Comprehensibility introduced by
70% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
70
    //    }
71
72
    /**
73
     * create a new user.
74
     *
75
     * @param bool $isNew Flag the new objects as "new"?
76
     *
77
     * @return \XoopsObject
78
     */
79
    public function create($isNew = true)
80
    {
81
        $obj = new $this->className();
82
        if (true === $isNew) {
83
            $obj->setNew();
84
        }
85
86
        return $obj;
87
    }
88
89
    /**
90
     * retrieve an object.
91
     *
92
     * @param mixed $id ID of the object - or array of ids for joint keys. Joint keys MUST be given in the same order as in the constructor
93
     * @param null  $fields
94
     * @param bool  $as_object
95
     *
96
     * @return mixed reference to the object, FALSE if failed
97
     *
98
     * @internal param bool $asObject whether to return an object or an array
99
     */
100
    public function get($id = null, $fields = null, $as_object = true) //get($id, $as_object = true)
0 ignored issues
show
Unused Code Comprehensibility introduced by
55% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
101
    {
102
        if (is_array($this->keyName)) {
103
            $criteria =  new \CriteriaCompo();
104
            for ($i = 0, $iMax = count($this->keyName); $i < $iMax; ++$i) {
105
                $criteria->add( new \Criteria($this->keyName[$i], (int)$id[$i]));
106
            }
107
        } else {
108
            $criteria =  new \Criteria($this->keyName, (int)$id);
109
        }
110
        $criteria->setLimit(1);
111
        $objectArray = $this->getObjects($criteria, false, true);
112
        if (1 != count($objectArray)) {
113
            return $this->create();
114
        }
115
116
        return $objectArray[0];
117
    }
118
119
    /**
120
     * retrieve objects from the database.
121
     *
122
     * @param \CriteriaElement $criteria {@link CriteriaElement} conditions to be met
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be null|\CriteriaElement?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
123
     * @param bool            $idAsKey  use the ID as key for the array?
124
     * @param bool            $asObject return an array of objects?
125
     *
126
     * @return array
127
     */
128
    public function &getObjects(\CriteriaElement $criteria = null, $idAsKey = false, $asObject = true)
129
    {
130
        $ret   = [];
131
        $limit = $start = 0;
132
        $sql   = 'SELECT * FROM ' . $this->table;
133 View Code Duplication
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
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...
134
            $sql .= ' ' . $criteria->renderWhere();
135
            if ('' != $criteria->getSort()) {
136
                $sql .= ' ORDER BY ' . $criteria->getSort() . ' ' . $criteria->getOrder();
137
            }
138
            $limit = $criteria->getLimit();
139
            $start = $criteria->getStart();
140
        }
141
        $result = $this->db->query($sql, $limit, $start);
142
        if (!$result) {
143
            return $ret;
144
        }
145
146
        $ret = $this->convertResultSet($result, $idAsKey, $asObject);
147
148
        return $ret;
149
    }
150
151
    /**
152
     * Convert a database resultset to a returnable array.
153
     *
154
     * @param XoopsObject $result  database resultset
155
     * @param bool        $idAsKey - should NOT be used with joint keys
156
     * @param bool        $asObject
157
     *
158
     * @return array
159
     */
160
    public function convertResultSet($result, $idAsKey = false, $asObject = true)
161
    {
162
        $ret = [];
163
        while ($myrow = $this->db->fetchArray($result)) {
164
            $obj = $this->create(false);
165
            $obj->assignVars($myrow);
166
            if (!$idAsKey) {
167 View Code Duplication
                if ($asObject) {
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...
168
                    $ret[] = $obj;
169
                } else {
170
                    $row  = [];
171
                    $vars =& $obj->getVars();
172
                    foreach (array_keys($vars) as $i) {
173
                        $row[$i] = $obj->getVar($i);
174
                    }
175
                    $ret[] = $row;
176
                }
177
            } else {
178
                if ($asObject) {
179
                    $ret[$myrow[$this->keyName]] = $obj;
180 View Code Duplication
                } else {
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...
181
                    $row  = [];
182
                    $vars =& $obj->getVars();
183
                    foreach (array_keys($vars) as $i) {
184
                        $row[$i] = $obj->getVar($i);
185
                    }
186
                    $ret[$myrow[$this->keyName]] = $row;
187
                }
188
            }
189
            unset($obj);
190
        }
191
192
        return $ret;
193
    }
194
195
    /**
196
     * Retrieve a list of objects as arrays - DON'T USE WITH JOINT KEYS.
197
     *
198
     * @param \CriteriaElement $criteria {@link CriteriaElement} conditions to be met
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be null|\CriteriaElement?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
199
     * @param int             $limit    Max number of objects to fetch
200
     * @param int             $start    Which record to start at
201
     *
202
     * @return array
203
     */
204
    public function getList(\CriteriaElement $criteria = null, $limit = 0, $start = 0)
205
    {
206
        $ret = [];
207
        if (null === $criteria) {
208
            $criteria =  new \CriteriaCompo();
209
        }
210
211
        if ('' == $criteria->getSort()) {
212
            $criteria->setSort($this->identifierName);
213
        }
214
215
        $sql = 'SELECT ' . $this->keyName;
216
        if (!empty($this->identifierName)) {
217
            $sql .= ', ' . $this->identifierName;
218
        }
219
        $sql .= ' FROM ' . $this->table;
220 View Code Duplication
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
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...
221
            $sql .= ' ' . $criteria->renderWhere();
222
            if ('' != $criteria->getSort()) {
223
                $sql .= ' ORDER BY ' . $criteria->getSort() . ' ' . $criteria->getOrder();
224
            }
225
            $limit = $criteria->getLimit();
226
            $start = $criteria->getStart();
227
        }
228
        $result = $this->db->query($sql, $limit, $start);
229
        if (!$result) {
230
            return $ret;
231
        }
232
233
        $myts = \MyTextSanitizer::getInstance();
234
        while ($myrow = $this->db->fetchArray($result)) {
235
            //identifiers should be textboxes, so sanitize them like that
236
            $ret[$myrow[$this->keyName]] = empty($this->identifierName) ? 1 : $myts->htmlSpecialChars($myrow[$this->identifierName]);
237
        }
238
239
        return $ret;
240
    }
241
242
    /**
243
     * count objects matching a condition.
244
     *
245
     * @param \CriteriaElement $criteria {@link CriteriaElement} to match
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be null|\CriteriaElement?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
246
     *
247
     * @return int|array count of objects
248
     */
249 View Code Duplication
    public function getCount(\CriteriaElement $criteria = null)
0 ignored issues
show
Duplication introduced by
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...
250
    {
251
        $field   = '';
252
        $groupby = false;
253
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
254
            if ('' != $criteria->groupby) {
255
                $groupby = true;
256
                $field   = $criteria->groupby . ', '; //Not entirely secure unless you KNOW that no criteria's groupby clause is going to be mis-used
257
            }
258
        }
259
        $sql = 'SELECT ' . $field . 'COUNT(*) FROM ' . $this->table;
260
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
261
            $sql .= ' ' . $criteria->renderWhere();
262
            if ('' != $criteria->groupby) {
263
                $sql .= $criteria->getGroupby();
264
            }
265
        }
266
        $result = $this->db->query($sql);
267
        if (!$result) {
268
            return 0;
269
        }
270
        if (false === $groupby) {
271
            list($count) = $this->db->fetchRow($result);
272
273
            return $count;
274
        } else {
275
            $ret = [];
276
            while (list($id, $count) = $this->db->fetchRow($result)) {
277
                $ret[$id] = $count;
278
            }
279
280
            return $ret;
281
        }
282
    }
283
284
    /**
285
     * delete an object from the database by id.
286
     *
287
     * @param mixed $id id of the object to delete
288
     * @param bool  $force
289
     *
290
     * @return bool FALSE if failed.
291
     */
292
    public function deleteById($id, $force = false) //delete(\XoopsObject $object, $force = false)
0 ignored issues
show
Unused Code Comprehensibility introduced by
47% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
293
    {
294
        if (is_array($this->keyName)) {
295
            $clause = [];
296
            for ($i = 0, $iMax = count($this->keyName); $i < $iMax; ++$i) {
297
                $clause[] = $this->keyName[$i] . ' = ' . $id[$i];
298
            }
299
            $whereclause = implode(' AND ', $clause);
300
        } else {
301
            $whereclause = $this->keyName . ' = ' . $id;
302
        }
303
        $sql = 'DELETE FROM ' . $this->table . ' WHERE ' . $whereclause;
304 View Code Duplication
        if (false !== $force) {
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...
305
            $result = $this->db->queryF($sql);
306
        } else {
307
            $result = $this->db->query($sql);
308
        }
309
        if (!$result) {
310
            return false;
311
        }
312
313
        return true;
314
    }
315
316
    /**
317
     * insert a new object in the database.
318
     *
319
     * @param XoopsObject $obj         reference to the object
0 ignored issues
show
Documentation introduced by
Should the type for parameter $obj not be \XoopsObject?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
320
     * @param bool        $force       whether to force the query execution despite security settings
321
     * @param bool        $checkObject check if the object is dirty and clean the attributes
322
     *
323
     * @return bool FALSE if failed, TRUE if already present and unchanged or successful
324
     */
325
    public function insert(\XoopsObject $obj, $force = false, $checkObject = true)
326
    {
327
        if (false !== $checkObject) {
328
            if (!is_object($obj)) {
329
                //                var_dump($obj);
0 ignored issues
show
Unused Code Comprehensibility introduced by
67% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
330
                return false;
331
            }
332
333
            if (!($obj instanceof $this->className && class_exists($this->className))) {
334
                $obj->setErrors(get_class($obj) . ' Differs from ' . $this->className);
335
336
                return false;
337
            }
338
        }
339
        if (!$obj->cleanVars()) {
340
            return false;
341
        }
342
343
        foreach ($obj->cleanVars as $k => $v) {
344
            if (XOBJ_DTYPE_INT == $obj->vars[$k]['data_type']) {
345
                $cleanvars[$k] = (int)$v;
0 ignored issues
show
Coding Style Comprehensibility introduced by
$cleanvars was never initialized. Although not strictly required by PHP, it is generally a good practice to add $cleanvars = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
346
            } elseif (is_array($v)) {
347
                $cleanvars[$k] = $this->db->quoteString(implode(',', $v));
0 ignored issues
show
Bug introduced by
The variable $cleanvars 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

  1. 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;
        }
    }
    
  2. 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;
    }
    
  3. 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...
348
            } else {
349
                $cleanvars[$k] = $this->db->quoteString($v);
350
            }
351
        }
352
        if ($obj->isNew()) {
353
            if (!is_array($this->keyName)) {
354
                if ($cleanvars[$this->keyName] < 1) {
355
                    $cleanvars[$this->keyName] = $this->db->genId($this->table . '_' . $this->keyName . '_seq');
356
                }
357
            }
358
            $sql = 'INSERT INTO ' . $this->table . ' (' . implode(',', array_keys($cleanvars)) . ') VALUES (' . implode(',', array_values($cleanvars)) . ')';
359
        } else {
360
            $sql = 'UPDATE ' . $this->table . ' SET';
361
            foreach ($cleanvars as $key => $value) {
362
                if ((!is_array($this->keyName) && $key == $this->keyName)
363
                    || (is_array($this->keyName)
364
                        && in_array($key, $this->keyName))) {
365
                    continue;
366
                }
367
                if (isset($notfirst)) {
368
                    $sql .= ',';
369
                }
370
                $sql      .= ' ' . $key . ' = ' . $value;
371
                $notfirst = true;
372
            }
373
            if (is_array($this->keyName)) {
374
                $whereclause = '';
375
                for ($i = 0, $iMax = count($this->keyName); $i < $iMax; ++$i) {
376
                    if ($i > 0) {
377
                        $whereclause .= ' AND ';
378
                    }
379
                    $whereclause .= $this->keyName[$i] . ' = ' . $obj->getVar($this->keyName[$i]);
380
                }
381
            } else {
382
                $whereclause = $this->keyName . ' = ' . $obj->getVar($this->keyName);
383
            }
384
            $sql .= ' WHERE ' . $whereclause;
385
        }
386 View Code Duplication
        if (false !== $force) {
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...
387
            $result = $this->db->queryF($sql);
388
        } else {
389
            $result = $this->db->query($sql);
390
        }
391
        if (!$result) {
392
            return false;
393
        }
394
        if (!is_array($this->keyName) && $obj->isNew()) {
395
            $obj->assignVar($this->keyName, $this->db->getInsertId());
396
        }
397
398
        return true;
399
    }
400
401
    /**
402
     * Change a value for objects with a certain criteria.
403
     *
404
     * @param string          $fieldname  Name of the field
405
     * @param string|array    $fieldvalue Value to write
406
     * @param CriteriaElement $criteria   {@link CriteriaElement}
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be null|\CriteriaElement?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
407
     * @param bool            $force
408
     *
409
     * @return bool
410
     */
411
    public function updateAll($fieldname, $fieldvalue, \CriteriaElement $criteria = null, $force = false)
412
    {
413
        $setClause = $fieldname . ' = ';
414
        if (is_numeric($fieldvalue)) {
415
            $setClause .= $fieldvalue;
416
        } elseif (is_array($fieldvalue)) {
417
            $setClause .= $this->db->quoteString(implode(',', $fieldvalue));
418
        } else {
419
            $setClause .= $this->db->quoteString($fieldvalue);
420
        }
421
        $sql = 'UPDATE ' . $this->table . ' SET ' . $setClause;
422
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
423
            $sql .= ' ' . $criteria->renderWhere();
424
        }
425 View Code Duplication
        if (false !== $force) {
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...
426
            $result = $this->db->queryF($sql);
427
        } else {
428
            $result = $this->db->query($sql);
429
        }
430
        if (!$result) {
431
            return false;
432
        }
433
434
        return true;
435
    }
436
437
    /**
438
     * @param      $fieldname
439
     * @param      $fieldvalue
440
     * @param null $criteria
441
     * @param bool $force
442
     *
443
     * @return bool
444
     */
445
    public function updateFieldValue($fieldname, $fieldvalue, $criteria = null, $force = true)
446
    {
447
        $sql = 'UPDATE ' . $this->table . ' SET ' . $fieldname . ' = ' . $fieldvalue;
448
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
449
            $sql .= ' ' . $criteria->renderWhere();
450
        }
451 View Code Duplication
        if (false !== $force) {
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...
452
            $result = $this->db->queryF($sql);
453
        } else {
454
            $result = $this->db->query($sql);
455
        }
456
        if (!$result) {
457
            return false;
458
        }
459
460
        return true;
461
    }
462
463
    /**
464
     * delete all objects meeting the conditions.
465
     *
466
     * @param CriteriaElement $criteria        {@link CriteriaElement}
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be null|\CriteriaElement?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
467
     *                                         with conditions to meet
468
     * @param bool            $force
469
     * @param bool            $asObject
470
     *
471
     * @return bool
472
     */
473
    public function deleteAll(\CriteriaElement $criteria = null, $force = true, $asObject = false)
474
    {
475
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
476
            $sql = 'DELETE FROM ' . $this->table;
477
            $sql .= ' ' . $criteria->renderWhere();
478
            if (!$this->db->query($sql)) {
479
                return false;
480
            }
481
            $rows = $this->db->getAffectedRows();
482
483
            return $rows > 0 ? $rows : true;
484
        }
485
486
        return false;
487
    }
488
489
    /**
490
     * @param $data
491
     *
492
     * @return array
0 ignored issues
show
Documentation introduced by
Should the return type not be array|object?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
493
     */
494
    public function _toObject($data)
495
    {
496
        if (is_array($data)) {
497
            $ret = [];
498
            foreach ($data as $v) {
499
                $object = new $this->className();
500
                $object->assignVars($v);
501
                $ret[] = $object;
502
            }
503
504
            return $ret;
505
        } else {
506
            $object = new $this->className();
507
            $object->assignVars($v);
0 ignored issues
show
Bug introduced by
The variable $v seems only to be defined at a later point. Did you maybe move this code here without moving the variable definition?

This error can happen if you refactor code and forget to move the variable initialization.

Let’s take a look at a simple example:

function someFunction() {
    $x = 5;
    echo $x;
}

The above code is perfectly fine. Now imagine that we re-order the statements:

function someFunction() {
    echo $x;
    $x = 5;
}

In that case, $x would be read before it is initialized. This was a very basic example, however the principle is the same for the found issue.

Loading history...
508
509
            return $object;
510
        }
511
    }
512
513
    /**
514
     * @param        $objects
515
     * @param array  $externalKeys
516
     * @param string $format
517
     *
518
     * @return array
519
     */
520
    public function objectToArray($objects, $externalKeys = [], $format = 's')
521
    {
522
        static $cache;
523
        if (!is_array($externalKeys)) {
524
            $externalKeys = [$externalKeys];
525
        } //JJD
526
527
        $ret = [];
528
        if (is_array($objects)) {
529
            $i = 0;
530
            foreach ($objects as $object) {
531
                $vars = $object->getVars();
532
                foreach ($vars as $k => $v) {
533
                    $ret[$i][$k] = $object->getVar($k, $format);
534
                }
535
                foreach ($externalKeys as $key) {
536
                    // Replace external key by corresponding object
537
                    $externalKey = $object->getExternalKey($key);
538
                    if (0 != $ret[$i][$key]) {
539
                        // Retrieving data if isn't cached
540
                        if (!isset($cached[$externalKey['keyName']][$ret[$i][$key]])) {
541 View Code Duplication
                            if ($externalKey['core']) {
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...
542
                                $handler = xoops_getHandler($externalKey['className']);
543
                            } else {
544
                                $handler = Extcal\Helper::getInstance()->getHandler($externalKey['className']);
545
                            }
546
                            $getMethod                                       = $externalKey['getMethodeName'];
547
                            $cached[$externalKey['keyName']][$ret[$i][$key]] = $this->objectToArrayWithoutExternalKey($handler->$getMethod($ret[$i][$key], true), $format);
0 ignored issues
show
Coding Style Comprehensibility introduced by
$cached was never initialized. Although not strictly required by PHP, it is generally a good practice to add $cached = array(); before regardless.

Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code.

Let’s take a look at an example:

foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}

As you can see in this example, the array $myArray is initialized the first time when the foreach loop is entered. You can also see that the value of the bar key is only written conditionally; thus, its value might result from a previous iteration.

This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization $myArray = array() either outside or inside the foreach loop.

Loading history...
548
                        }
549
                        $ret[$i][$externalKey['keyName']] = $cached[$externalKey['keyName']][$ret[$i][$key]];
0 ignored issues
show
Bug introduced by
The variable $cached 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

  1. 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;
        }
    }
    
  2. 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;
    }
    
  3. 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...
550
                    }
551
                    unset($ret[$i][$key]);
552
                }
553
                ++$i;
554
            }
555
        } else {
556
            $vars = $objects->getVars();
557
            foreach ($vars as $k => $v) {
558
                $ret[$k] = $objects->getVar($k, $format);
559
            }
560
            foreach ($externalKeys as $key) {
561
                // Replace external key by corresponding object
562
                $externalKey = $objects->getExternalKey($key);
563
                if (0 != $ret[$key]) {
564
                    // Retriving data if isn't cached
565
                    if (!isset($cached[$externalKey['keyName']][$ret[$key]])) {
566 View Code Duplication
                        if ($externalKey['core']) {
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...
567
                            $handler = xoops_getHandler($externalKey['className']);
568
                        } else {
569
                            $handler = Extcal\Helper::getInstance()->getHandler($externalKey['className']);
570
                        }
571
                        $getMethod                                   = $externalKey['getMethodeName'];
572
                        $cached[$externalKey['keyName']][$ret[$key]] = $this->objectToArrayWithoutExternalKey($handler->$getMethod($ret[$key], true), $format);
573
                    }
574
                    $ret[$externalKey['keyName']] = $cached[$externalKey['keyName']][$ret[$key]];
575
                }
576
                unset($ret[$key]);
577
            }
578
        }
579
580
        return $ret;
581
    }
582
583
    /**
584
     * @param        $object
585
     * @param string $format
586
     *
587
     * @return array
588
     */
589
    public function objectToArrayWithoutExternalKey($object, $format = 's')
590
    {
591
        $ret = [];
592
        if (null !== $object) {
593
            $vars = $object->getVars();
594
            foreach ($vars as $k => $v) {
595
                $ret[$k] = $object->getVar($k, $format);
596
            }
597
        }
598
599
        return $ret;
600
    }
601
602
    /**
603
     * @param        $fieldname
604
     * @param        $criteria
605
     * @param string $op
606
     *
607
     * @return bool
608
     */
609
    public function updateCounter($fieldname, $criteria, $op = '+')
610
    {
611
        $sql    = 'UPDATE ' . $this->table . ' SET ' . $fieldname . ' = ' . $fieldname . $op . '1';
612
        $sql    .= ' ' . $criteria->renderWhere();
613
        $result = $this->db->queryF($sql);
614
        if (!$result) {
615
            return false;
616
        }
617
618
        return true;
619
    }
620
621
    /**
622
     * @param null|CriteriaElement $criteria
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be null|\CriteriaElement?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
623
     * @param string               $sum
624
     *
625
     * @return array|string
626
     */
627 View Code Duplication
    public function getSum(\CriteriaElement $criteria = null, $sum = '*')
0 ignored issues
show
Duplication introduced by
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...
628
    {
629
        $field   = '';
630
        $groupby = false;
631
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
632
            if ('' != $criteria->groupby) {
633
                $groupby = true;
634
                $field   = $criteria->groupby . ', '; //Not entirely secure unless you KNOW that no criteria's groupby clause is going to be mis-used
635
            }
636
        }
637
        $sql = 'SELECT ' . $field . "SUM($sum) FROM " . $this->table;
638
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
639
            $sql .= ' ' . $criteria->renderWhere();
640
            if ('' != $criteria->groupby) {
641
                $sql .= $criteria->getGroupby();
642
            }
643
        }
644
        $result = $this->db->query($sql);
645
        if (!$result) {
646
            return 0;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return 0; (integer) is incompatible with the return type documented by XoopsModules\Extcal\Extc...leObjectHandler::getSum of type array|string.

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...
647
        }
648
        if (false === $groupby) {
649
            list($sum) = $this->db->fetchRow($result);
650
651
            return $sum;
652
        } else {
653
            $ret = [];
654
            while (list($id, $sum) = $this->db->fetchRow($result)) {
655
                $ret[$id] = $sum;
656
            }
657
658
            return $ret;
659
        }
660
    }
661
662
    /**
663
     * @param null|CriteriaElement $criteria
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be null|\CriteriaElement?

This check looks for @param annotations where the type inferred by our type inference engine differs from the declared type.

It makes a suggestion as to what type it considers more descriptive.

Most often this is a case of a parameter that can be null in addition to its declared types.

Loading history...
664
     * @param string               $max
665
     *
666
     * @return array|string
667
     */
668 View Code Duplication
    public function getMax(\CriteriaElement $criteria = null, $max = '*')
0 ignored issues
show
Duplication introduced by
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...
669
    {
670
        $field   = '';
671
        $groupby = false;
672
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
673
            if ('' != $criteria->groupby) {
674
                $groupby = true;
675
                $field   = $criteria->groupby . ', '; //Not entirely secure unless you KNOW that no criteria's groupby clause is going to be mis-used
676
            }
677
        }
678
        $sql = 'SELECT ' . $field . "MAX($max) FROM " . $this->table;
679
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
680
            $sql .= ' ' . $criteria->renderWhere();
681
            if ('' != $criteria->groupby) {
682
                $sql .= $criteria->getGroupby();
683
            }
684
        }
685
        $result = $this->db->query($sql);
686
        if (!$result) {
687
            return 0;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return 0; (integer) is incompatible with the return type documented by XoopsModules\Extcal\Extc...leObjectHandler::getMax of type array|string.

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...
688
        }
689
        if (false === $groupby) {
690
            list($max) = $this->db->fetchRow($result);
691
692
            return $max;
693
        } else {
694
            $ret = [];
695
            while (list($id, $max) = $this->db->fetchRow($result)) {
696
                $ret[$id] = $max;
697
            }
698
699
            return $ret;
700
        }
701
    }
702
703
    /**
704
     * @param null   $criteria
705
     * @param string $avg
706
     *
707
     * @return int
708
     */
709
    public function getAvg($criteria = null, $avg = '*')
710
    {
711
        $field = '';
712
713
        $sql = 'SELECT ' . $field . "AVG($avg) FROM " . $this->table;
714
        if (isset($criteria) && is_subclass_of($criteria, 'CriteriaElement')) {
0 ignored issues
show
Bug introduced by
Due to PHP Bug #53727, is_subclass_of returns inconsistent results on some PHP versions for interfaces; you could instead use ReflectionClass::implementsInterface.
Loading history...
715
            $sql .= ' ' . $criteria->renderWhere();
716
        }
717
        $result = $this->db->query($sql);
718
        if (!$result) {
719
            return 0;
720
        }
721
        list($sum) = $this->db->fetchRow($result);
722
723
        return $sum;
724
    }
725
726
    /**
727
     * @return mixed
728
     */
729
    public function getInsertId()
730
    {
731
        return $this->db->getInsertId();
732
    }
733
}
734