Completed
Push — master ( 454ebd...de22fe )
by Michael
03:58
created

FaqHandler::getFaqsFromSearch()   F

Complexity

Conditions 22
Paths 12296

Size

Total Lines 123
Code Lines 74

Duplication

Lines 6
Ratio 4.88 %

Importance

Changes 0
Metric Value
cc 22
eloc 74
nc 12296
nop 5
dl 6
loc 123
rs 2
c 0
b 0
f 0

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 namespace XoopsModules\Smartfaq;
2
3
/**
4
 * Module: SmartFAQ
5
 * Author: The SmartFactory <www.smartfactory.ca>
6
 * Licence: GNU
7
 */
8
9
use XoopsModules\Smartfaq;
10
use XoopsModules\Smartfaq\Constants;
11
12
// defined('XOOPS_ROOT_PATH') || die('Restricted access');
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...
13
14
//require_once XOOPS_ROOT_PATH . '/modules/smartfaq/class/category.php';
0 ignored issues
show
Unused Code Comprehensibility introduced by
38% 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...
15
16
17
/**
18
 * Q&A handler class.
19
 * This class is responsible for providing data access mechanisms to the data source
20
 * of Q&A class objects.
21
 *
22
 * @author  marcan <[email protected]>
23
 * @package SmartFAQ
24
 */
25
class FaqHandler extends \XoopsObjectHandler
26
{
27
    /**
28
     * @param  bool $isNew
29
     * @return Smartfaq\Faq
30
     */
31
    public function create($isNew = true)
32
    {
33
        $faq = new Smartfaq\Faq();
34
        if ($isNew) {
35
            $faq->setDefaultPermissions();
36
            $faq->setNew();
37
        }
38
39
        return $faq;
40
    }
41
42
    /**
43
     * retrieve an FAQ
44
     *
45
     * @param  int $id faqid of the user
46
     * @return mixed reference to the {@link Smartfaq\Faq} object, FALSE if failed
47
     */
48 View Code Duplication
    public function get($id)
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...
49
    {
50
        if ((int)$id > 0) {
51
            $sql = 'SELECT * FROM ' . $this->db->prefix('smartfaq_faq') . ' WHERE faqid=' . $id;
52
            if (!$result = $this->db->query($sql)) {
53
                return false;
54
            }
55
56
            $numrows = $this->db->getRowsNum($result);
57
            if (1 == $numrows) {
58
                $faq = new Smartfaq\Faq();
59
                $faq->assignVars($this->db->fetchArray($result));
60
61
                return $faq;
62
            }
63
        }
64
65
        return false;
66
    }
67
68
    /**
69
     * insert a new faq in the database
70
     *
71
     * @param \XoopsObject $faq reference to the {@link Smartfaq\Faq}
72
     *                          object
73
     * @param  bool        $force
74
     * @return bool        FALSE if failed, TRUE if already present and unchanged or successful
75
     */
76
    public function insert(\XoopsObject $faq, $force = false)
77
    {
78
        if ('xoopsmodules\smartfaq\faq' !== strtolower(get_class($faq))) {
79
            return false;
80
        }
81
82
        if (!$faq->isDirty()) {
83
            return true;
84
        }
85
86
        if (!$faq->cleanVars()) {
87
            return false;
88
        }
89
90
        foreach ($faq->cleanVars as $k => $v) {
91
            ${$k} = $v;
92
        }
93
94
        if ($faq->isNew()) {
95
            $sql = sprintf(
96
                'INSERT INTO %s (faqid, categoryid, question, howdoi, diduno, uid, datesub, `status`, counter, weight, html, smiley, xcodes, cancomment, comments, notifypub, modulelink, contextpage, exacturl, partialview) VALUES (NULL, %u, %s, %s, %s, %u, %u, %u, %u, %u, %u, %u, %u, %u, %u, %u, %s, %s, %u, %u)',
97
                           $this->db->prefix('smartfaq_faq'),
98
                $categoryid,
0 ignored issues
show
Bug introduced by
The variable $categoryid does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
99
                $this->db->quoteString($question),
0 ignored issues
show
Bug introduced by
The variable $question does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
100
                $this->db->quoteString($howdoi),
0 ignored issues
show
Bug introduced by
The variable $howdoi does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
101
                $this->db->quoteString($diduno),
0 ignored issues
show
Bug introduced by
The variable $diduno does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
102
                $uid,
0 ignored issues
show
Bug introduced by
The variable $uid does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
103
                time(),
104
                $status,
0 ignored issues
show
Bug introduced by
The variable $status does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
105
                $counter,
0 ignored issues
show
Bug introduced by
The variable $counter does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
106
                $weight,
0 ignored issues
show
Bug introduced by
The variable $weight does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
107
                $html,
0 ignored issues
show
Bug introduced by
The variable $html does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
108
                $smiley,
0 ignored issues
show
Bug introduced by
The variable $smiley does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
109
                $xcodes,
0 ignored issues
show
Bug introduced by
The variable $xcodes does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
110
                $cancomment,
0 ignored issues
show
Bug introduced by
The variable $cancomment does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
111
                $comments,
0 ignored issues
show
Bug introduced by
The variable $comments does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
112
                $notifypub,
0 ignored issues
show
Bug introduced by
The variable $notifypub does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
113
                $this->db->quoteString($modulelink),
0 ignored issues
show
Bug introduced by
The variable $modulelink does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
114
                $this->db->quoteString($contextpage),
0 ignored issues
show
Bug introduced by
The variable $contextpage does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
115
                $exacturl,
0 ignored issues
show
Bug introduced by
The variable $exacturl does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
116
                $partialview
0 ignored issues
show
Bug introduced by
The variable $partialview does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
117
            );
118
        } else {
119
            $sql = sprintf(
120
                'UPDATE %s SET categoryid = %u, question = %s, howdoi = %s, diduno = %s, uid = %u, datesub = %u, `status` = %u, counter = %u, weight = %u, html = %u, smiley = %u, xcodes = %u, cancomment = %u, comments = %u, notifypub = %u, modulelink = %s, contextpage = %s, exacturl = %u, partialview = %u  WHERE faqid = %u',
121
122
                $this->db->prefix('smartfaq_faq'),
123
                $categoryid,
124
                $this->db->quoteString($question),
125
                $this->db->quoteString($howdoi),
126
                $this->db->quoteString($diduno),
127
                $uid,
128
                $datesub,
0 ignored issues
show
Bug introduced by
The variable $datesub does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
129
                $status,
130
                $counter,
131
                $weight,
132
                $html,
133
                $smiley,
134
                $xcodes,
135
                $cancomment,
136
                $comments,
137
                $notifypub,
138
                $this->db->quoteString($modulelink),
139
                $this->db->quoteString($contextpage),
140
                $exacturl,
141
                $partialview,
142
                $faqid
0 ignored issues
show
Bug introduced by
The variable $faqid does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
143
            );
144
        }
145 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...
146
            $result = $this->db->queryF($sql);
147
        } else {
148
            $result = $this->db->query($sql);
149
        }
150
151
        if (!$result) {
152
            $faq->setErrors('Could not store data in the database.<br />'.$this->db->error().' ('.$this->db->errno().')<br />'.$sql);
153
154
            $logger = \XoopsLogger::getInstance();
155
            $logger->handleError(E_USER_WARNING, $sql, __FILE__, __LINE__);
156
            $logger->addExtra('Token Validation', 'No valid token found in request/session');
157
158
159
            /** @var Smartfaq\Helper $helper */
160
            $helper = Smartfaq\Helper::getInstance();
161
            $helper->addLog($this->db->error());
162
163
            /** @var \XoopsObject $faq */
164
//            $faq->setError($this->db->error());
0 ignored issues
show
Unused Code Comprehensibility introduced by
72% 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...
165
166
167
            trigger_error('Class ' . $faq . ' could not be saved ' . __FILE__ . ' at line ' . __LINE__, E_USER_WARNING);
168
169
            return false;
170
        }
171
172
        if ($faq->isNew()) {
173
            $faq->assignVar('faqid', $this->db->getInsertId());
174
        }
175
176
        // Saving permissions
177
        Smartfaq\Utility::saveItemPermissions($faq->getGroups_read(), $faq->faqid());
178
179
        return true;
180
    }
181
182
    /**
183
     * delete an FAQ from the database
184
     *
185
     * @param \XoopsObject $faq reference to the FAQ to delete
186
     * @param  bool        $force
187
     * @return bool        FALSE if failed.
188
     */
189
    public function delete(\XoopsObject $faq, $force = false)
190
    {
191
        $smartModule = Smartfaq\Utility::getModuleInfo();
192
        $module_id   = $smartModule->getVar('mid');
193
194
        if ('\'xoopsmodules\smartfaq\faq' !== strtolower(get_class($faq))) {
195
            return false;
196
        }
197
198
        // Deleting the answers
199
        $answerHandler = new Smartfaq\AnswerHandler($this->db);
200
        if (!$answerHandler->deleteFaqAnswers($faq)) {
201
            // error msg...
202
            echo 'error while deleteing an answer';
203
        }
204
205
        $sql = sprintf('DELETE FROM %s WHERE faqid = %u', $this->db->prefix('smartfaq_faq'), $faq->getVar('faqid'));
206
207 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...
208
            $result = $this->db->queryF($sql);
209
        } else {
210
            $result = $this->db->query($sql);
211
        }
212
        if (!$result) {
213
            return false;
214
        }
215
216
        xoops_groupperm_deletebymoditem($module_id, 'item_read', $faq->faqid());
217
218
        return true;
219
    }
220
221
    /**
222
     * retrieve FAQs from the database
223
     *
224
     * @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...
225
     * @param  bool            $id_as_key use the faqid as key for the array?
226
     * @param  string          $notNullFields
227
     * @return false|array  array of <a href='psi_element://Smartfaq\Faq'>Smartfaq\Faq</a> objects
228
     */
229
    public function &getObjects(\CriteriaElement $criteria = null, $id_as_key = false, $notNullFields = '')
0 ignored issues
show
Coding Style introduced by
getObjects uses the super-global variable $GLOBALS which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
230
    {
231
        $ret   = [];
232
        $limit = $start = 0;
233
        $sql   = 'SELECT * FROM ' . $this->db->prefix('smartfaq_faq');
234
235 View Code Duplication
        if (null !== $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...
236
            $whereClause = $criteria->renderWhere();
237
238
            if ('WHERE ()' !== $whereClause) {
239
                $sql .= ' ' . $criteria->renderWhere();
240
                if (!empty($notNullFields)) {
241
                    $sql .= $this->NotNullFieldClause($notNullFields, true);
242
                }
243
            } elseif (!empty($notNullFields)) {
244
                $sql .= ' WHERE ' . $this->NotNullFieldClause($notNullFields);
245
            }
246
            if ('' != $criteria->getSort()) {
247
                $sql .= ' ORDER BY ' . $criteria->getSort() . ' ' . $criteria->getOrder();
248
            }
249
            $limit = $criteria->getLimit();
250
            $start = $criteria->getStart();
251
        } elseif (!empty($notNullFields)) {
252
            $sql .= $sql .= ' WHERE ' . $this->NotNullFieldClause($notNullFields);
253
        }
254
255
        //echo "<br>" . $sql . "<br>";
0 ignored issues
show
Unused Code Comprehensibility introduced by
42% 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...
256
        $result = $this->db->query($sql, $limit, $start);
257
        if (!$result) {
258
            return false;
259
        }
260
261
        if (0 == $GLOBALS['xoopsDB']->getRowsNum($result)) {
262
            $temp = false;
263
            return $temp;
264
        }
265
266
        while (false !== ($myrow = $this->db->fetchArray($result))) {
267
            $faq = new Smartfaq\Faq();
268
            $faq->assignVars($myrow);
269
270
            if (!$id_as_key) {
271
                $ret[] =& $faq;
272
            } else {
273
                $ret[$myrow['faqid']] =& $faq;
274
            }
275
            unset($faq);
276
        }
277
278
        return $ret;
279
    }
280
281
    /**
282
     * @param  null|\CriteriaElement $criteria
283
     * @param  bool                 $id_as_key
284
     * @param  string               $notNullFields
285
     * @return array|bool
286
     */
287
    public function getObjectsAdminSide(\CriteriaElement $criteria = null, $id_as_key = false, $notNullFields = '')
0 ignored issues
show
Coding Style introduced by
getObjectsAdminSide uses the super-global variable $GLOBALS which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
288
    {
289
        $ret   = [];
290
        $limit = $start = 0;
291
        $sql   = 'SELECT
292
                            faq.faqid AS faqid,
293
                            faq.categoryid AS categoryid,
294
                            faq.question AS question,
295
                            faq.howdoi AS howdoi,
296
                            faq.diduno AS diduno,
297
                            faq.uid AS uid,
298
                            faq.datesub AS datesub,
299
                            faq.status AS status,
300
                            faq.counter AS counter,
301
                            faq.weight AS weight,
302
                            faq.html AS html,
303
                            faq.smiley AS smiley,
304
                            faq.image AS image,
305
                            faq.linebreak AS linebreak,
306
                            faq.xcodes AS xcodes,
307
                            faq.cancomment AS cancomment,
308
                            faq.comments AS comments,
309
                            faq.notifypub AS notifypub,
310
                            faq.modulelink AS modulelink,
311
                            faq.contextpage AS contextpage,
312
                            faq.exacturl AS exacturl
313
                FROM ' . $this->db->prefix('smartfaq_faq') . ' AS faq INNER JOIN ' . $this->db->prefix('smartfaq_categories') . ' AS category ON faq.categoryid = category.categoryid ';
314
315 View Code Duplication
        if (null !== $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...
316
            $whereClause = $criteria->renderWhere();
317
318
            if ('WHERE ()' !== $whereClause) {
319
                $sql .= ' ' . $criteria->renderWhere();
320
                if (!empty($notNullFields)) {
321
                    $sql .= $this->NotNullFieldClause($notNullFields, true);
322
                }
323
            } elseif (!empty($notNullFields)) {
324
                $sql .= ' WHERE ' . $this->NotNullFieldClause($notNullFields);
325
            }
326
            if ('' != $criteria->getSort()) {
327
                $sql .= ' ORDER BY ' . $criteria->getSort() . ' ' . $criteria->getOrder();
328
            }
329
            $limit = $criteria->getLimit();
330
            $start = $criteria->getStart();
331
        } elseif (!empty($notNullFields)) {
332
            $sql .= $sql .= ' WHERE ' . $this->NotNullFieldClause($notNullFields);
333
        }
334
335
        //echo "<br>" . $sql . "<br>";
0 ignored issues
show
Unused Code Comprehensibility introduced by
42% 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...
336
        $result = $this->db->query($sql, $limit, $start);
337
        if (!$result) {
338
            return false;
339
        }
340
341
        if (0 == $GLOBALS['xoopsDB']->getRowsNum($result)) {
342
            return false;
343
        }
344
345
        while (false !== ($myrow = $this->db->fetchArray($result))) {
346
            $faq = new Smartfaq\Faq();
347
            $faq->assignVars($myrow);
348
349
            if (!$id_as_key) {
350
                $ret[] =& $faq;
351
            } else {
352
                $ret[$myrow['faqid']] =& $faq;
353
            }
354
            unset($faq);
355
        }
356
357
        return $ret;
358
359
        /*while ($myrow = $this->db->fetchArray($result)) {
0 ignored issues
show
Unused Code Comprehensibility introduced by
62% 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...
360
            $faq = new Smartfaq\Faq($myrow['faqid']);
361
362
            if (!$id_as_key) {
363
                $ret[] =& $faq;
364
            } else {
365
                $ret[$myrow['faqid']] =& $faq;
366
            }
367
            unset($faq);
368
        }
369
370
        return $ret;*/
371
    }
372
373
    /**
374
     * count FAQs matching a condition
375
     *
376
     * @param  object $criteria {@link CriteriaElement} to match
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be object|null?

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...
377
     * @param  string $notNullFields
378
     * @return int    count of FAQs
379
     */
380
    public function getCount($criteria = null, $notNullFields = '')
381
    {
382
        $sql = 'SELECT COUNT(*) FROM ' . $this->db->prefix('smartfaq_faq');
383
        if (null !== $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...
384
            $whereClause = $criteria->renderWhere();
385
            if ('WHERE ()' !== $whereClause) {
386
                $sql .= ' ' . $criteria->renderWhere();
387
                if (!empty($notNullFields)) {
388
                    $sql .= $this->NotNullFieldClause($notNullFields, true);
389
                }
390
            } elseif (!empty($notNullFields)) {
391
                $sql .= ' WHERE ' . $this->NotNullFieldClause($notNullFields);
392
            }
393
        } elseif (!empty($notNullFields)) {
394
            $sql .= ' WHERE ' . $this->NotNullFieldClause($notNullFields);
395
        }
396
397
        //echo "<br>" . $sql . "<br>";
0 ignored issues
show
Unused Code Comprehensibility introduced by
42% 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...
398
        $result = $this->db->query($sql);
399
        if (!$result) {
400
            return 0;
401
        }
402
        list($count) = $this->db->fetchRow($result);
403
404
        return $count;
405
    }
406
407
    /**
408
     * @param  int    $categoryid
409
     * @param  string $status
410
     * @param  string $notNullFields
411
     * @return int
412
     */
413
    public function getFaqsCount($categoryid = -1, $status = '', $notNullFields = '')
414
    {
415
        global $xoopsUser;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
416
417
        //  if ( ($categoryid = -1) && (empty($status) || ($status == -1)) ) {
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...
418
        //return $this->getCount();
0 ignored issues
show
Unused Code Comprehensibility introduced by
75% 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...
419
        //}
420
421
        $userIsAdmin = Smartfaq\Utility::userIsAdmin();
422
        // Categories for which user has access
423 View Code Duplication
        if (!$userIsAdmin) {
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...
424
            /** @var \XoopsModules\Smartfaq\PermissionHandler $smartPermHandler */
425
            $smartPermHandler = \XoopsModules\Smartfaq\Helper::getInstance()->getHandler('Permission');
426
427
            $categoriesGranted = $smartPermHandler->getPermissions('category');
428
            $grantedCategories = new \Criteria('categoryid', '(' . implode(',', $categoriesGranted) . ')', 'IN');
429
430
            $faqsGranted = $smartPermHandler->getPermissions('item');
431
            $grantedFaq  = new \CriteriaCompo();
432
            $grantedFaq->add(new \Criteria('faqid', '(' . implode(',', $faqsGranted) . ')', 'IN'), 'OR');
433
            // If user is anonymous, check if the FAQ allow partialview
434
            if (!is_object($xoopsUser)) {
435
                $grantedFaq->add(new \Criteria('partialview', '1'), 'OR');
436
            }
437
        }
438
439
        if (isset($categoryid) && (-1 != $categoryid)) {
440
            $criteriaCategory = new \Criteria('categoryid', $categoryid);
441
        }
442
443
        $criteriaStatus = new \CriteriaCompo();
444
        if (!empty($status) && is_array($status)) {
445
            foreach ($status as $v) {
446
                $criteriaStatus->add(new \Criteria('status', $v), 'OR');
447
            }
448
        } elseif (!empty($status) && (-1 != $status)) {
449
            $criteriaStatus->add(new \Criteria('status', $status), 'OR');
450
        }
451
452
        $criteriaPermissions = new \CriteriaCompo();
453
        if (!$userIsAdmin) {
454
            $criteriaPermissions->add($grantedCategories, 'AND');
0 ignored issues
show
Bug introduced by
The variable $grantedCategories 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...
455
            $criteriaPermissions->add($grantedFaq, 'AND');
0 ignored issues
show
Bug introduced by
The variable $grantedFaq 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...
456
        }
457
458
        $criteria = new \CriteriaCompo();
459
        if (!empty($criteriaCategory)) {
460
            $criteria->add($criteriaCategory);
461
        }
462
463
        if (!empty($criteriaPermissions) && (!$userIsAdmin)) {
464
            $criteria->add($criteriaPermissions);
465
        }
466
467
        if (!empty($criteriaStatus)) {
468
            $criteria->add($criteriaStatus);
469
        }
470
471
        return $this->getCount($criteria, $notNullFields);
472
    }
473
474
    /**
475
     * @return array
476
     */
477
    public function getFaqsCountByStatus()
478
    {
479
        $sql    = 'SELECT status, COUNT(*) FROM ' . $this->db->prefix('smartfaq_faq') . ' GROUP BY status';
480
        $result = $this->db->query($sql);
481
        if (!$result) {
482
            return [];
483
        }
484
        $ret = [];
485
        while (list($status, $count) = $this->db->fetchRow($result)) {
486
            $ret[$status] = $count;
487
        }
488
489
        return $ret;
490
    }
491
492
    /**
493
     * @param  int    $limit
494
     * @param  int    $start
495
     * @param  int    $categoryid
496
     * @param  string $sort
497
     * @param  string $order
498
     * @param  bool   $asobject
499
     * @return array
0 ignored issues
show
Documentation introduced by
Should the return type not be false|array?

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...
500
     */
501
    public function getAllPublished(
502
        $limit = 0,
503
        $start = 0,
504
        $categoryid = -1,
505
        $sort = 'datesub',
506
        $order = 'DESC',
507
        $asobject = true
508
    ) {
509
        return $this->getFaqs($limit, $start, [Constants::SF_STATUS_PUBLISHED, Constants::SF_STATUS_NEW_ANSWER], $categoryid, $sort, $order, null, $asobject, null);
510
    }
511
512
    /**
513
     * @param  int    $limit
514
     * @param  int    $start
515
     * @param  string $status
516
     * @param  int    $categoryid
517
     * @param  string $sort
518
     * @param  string $order
519
     * @param  string $notNullFields
520
     * @param  bool   $asobject
521
     * @param  null   $otherCriteria
522
     * @return array
0 ignored issues
show
Documentation introduced by
Should the return type not be false|array?

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...
523
     */
524
    public function getFaqs(
525
        $limit = 0,
526
        $start = 0,
527
        $status = '',
528
        $categoryid = -1,
529
        $sort = 'datesub',
530
        $order = 'DESC',
531
        $notNullFields = '',
532
        $asobject = true,
533
        $otherCriteria = null
534
    ) {
535
        global $xoopsUser;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
536
//        require_once XOOPS_ROOT_PATH . '/modules/smartfaq/include/functions.php';
537
538
        //if ( ($categoryid == -1) && (empty($status) || ($status == -1)) && ($limit == 0) && ($start ==0) ) {
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...
539
        //  return $this->getObjects();
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...
540
        //}
541
        $ret         = [];
542
        $userIsAdmin = Smartfaq\Utility::userIsAdmin();
543
        // Categories for which user has access
544 View Code Duplication
        if (!$userIsAdmin) {
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...
545
            /** @var \XoopsModules\Smartfaq\PermissionHandler $smartPermHandler */
546
            $smartPermHandler = \XoopsModules\Smartfaq\Helper::getInstance()->getHandler('Permission');
547
548
            $categoriesGranted = $smartPermHandler->getPermissions('category');
549
            $grantedCategories = new \Criteria('categoryid', '(' . implode(',', $categoriesGranted) . ')', 'IN');
550
551
            $faqsGranted = $smartPermHandler->getPermissions('item');
552
            $grantedFaq  = new \CriteriaCompo();
553
            $grantedFaq->add(new \Criteria('faqid', '(' . implode(',', $faqsGranted) . ')', 'IN'), 'OR');
554
            // If user is anonymous, check if the FAQ allow partialview
555
            if (!is_object($xoopsUser)) {
556
                $grantedFaq->add(new \Criteria('partialview', '1'), 'OR');
557
            }
558
        }
559
560
        if (isset($categoryid) && (-1 != $categoryid)) {
561
            if (is_array($categoryid)) {
562
                $criteriaCategory = new \Criteria('categoryid', '(' . implode(',', $categoryid) . ')', 'IN');
563
            } else {
564
                $criteriaCategory = new \Criteria('categoryid', (int)$categoryid);
565
            }
566
        }
567
568 View Code Duplication
        if (!empty($status) && is_array($status)) {
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...
569
            $criteriaStatus = new \CriteriaCompo();
570
            foreach ($status as $v) {
571
                $criteriaStatus->add(new \Criteria('status', $v), 'OR');
572
            }
573
        } elseif (!empty($status) && (-1 != $status)) {
574
            $criteriaStatus = new \CriteriaCompo();
575
            $criteriaStatus->add(new \Criteria('status', $status), 'OR');
576
        }
577
578
        $criteriaPermissions = new \CriteriaCompo();
579
        if (!$userIsAdmin) {
580
            $criteriaPermissions->add($grantedCategories, 'AND');
0 ignored issues
show
Bug introduced by
The variable $grantedCategories 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...
581
            $criteriaPermissions->add($grantedFaq, 'AND');
0 ignored issues
show
Bug introduced by
The variable $grantedFaq 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...
582
        }
583
584
        $criteria = new \CriteriaCompo();
585
        if (!empty($criteriaCategory)) {
586
            $criteria->add($criteriaCategory);
587
        }
588
589
        if (!empty($criteriaPermissions) && (!$userIsAdmin)) {
590
            $criteria->add($criteriaPermissions);
591
        }
592
593
        if (!empty($criteriaStatus)) {
594
            $criteria->add($criteriaStatus);
595
        }
596
597
        if (!empty($otherCriteria)) {
598
            $criteria->add($otherCriteria);
599
        }
600
601
        $criteria->setLimit($limit);
602
        $criteria->setStart($start);
603
        $criteria->setSort($sort);
604
        $criteria->setOrder($order);
605
        $ret =& $this->getObjects($criteria, false, $notNullFields);
606
607
        return $ret;
608
    }
609
610
    /**
611
     * @param  int    $limit
612
     * @param  int    $start
613
     * @param  string $status
614
     * @param  int    $categoryid
615
     * @param  string $sort
616
     * @param  string $order
617
     * @param  bool   $asobject
618
     * @param  null   $otherCriteria
619
     * @return array|bool
620
     */
621
    public function getFaqsAdminSide(
622
        $limit = 0,
623
        $start = 0,
624
        $status = '',
625
        $categoryid = -1,
626
        $sort = 'datesub',
627
        $order = 'DESC',
628
        $asobject = true,
629
        $otherCriteria = null
630
    ) {
631
//        require_once XOOPS_ROOT_PATH . '/modules/smartfaq/include/functions.php';
632
633
        $smartModule = Smartfaq\Utility::getModuleInfo();
0 ignored issues
show
Unused Code introduced by
$smartModule 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...
634
635
        $ret = [];
0 ignored issues
show
Unused Code introduced by
$ret 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...
636
637
        if (isset($categoryid) && (-1 != $categoryid)) {
638
            $criteriaCategory = new \Criteria('faq.categoryid', $categoryid);
639
        }
640
641 View Code Duplication
        if (!empty($status) && is_array($status)) {
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...
642
            $criteriaStatus = new \CriteriaCompo();
643
            foreach ($status as $v) {
644
                $criteriaStatus->add(new \Criteria('faq.status', $v), 'OR');
645
            }
646
        } elseif (!empty($status) && (-1 != $status)) {
647
            $criteriaStatus = new \CriteriaCompo();
648
            $criteriaStatus->add(new \Criteria('faq.status', $status), 'OR');
649
        }
650
651
        $criteria = new \CriteriaCompo();
652
        if (!empty($criteriaCategory)) {
653
            $criteria->add($criteriaCategory);
654
        }
655
656
        if (!empty($criteriaStatus)) {
657
            $criteria->add($criteriaStatus);
658
        }
659
660
        if (!empty($otherCriteria)) {
661
            $criteria->add($otherCriteria);
662
        }
663
664
        $criteria->setLimit($limit);
665
        $criteria->setStart($start);
666
        $criteria->setSort($sort);
667
        $criteria->setOrder($order);
668
        $ret = $this->getObjectsAdminSide($criteria, false);
669
670
        return $ret;
671
    }
672
673
    /**
674
     * @param  string $field
675
     * @param  string $status
676
     * @param  int    $category
677
     * @return bool|mixed
678
     */
679
    public function getRandomFaq($field = '', $status = '', $category = -1)
680
    {
681
        $ret = false;
682
683
        $notNullFields = $field;
684
685
        // Getting the number of published FAQ
686
        $totalFaqs = $this->getFaqsCount(-1, $status, $notNullFields);
687
688
        if ($totalFaqs > 0) {
689
            --$totalFaqs;
690
            mt_srand((double)microtime() * 1000000);
691
            $entrynumber = mt_rand(0, $totalFaqs);
692
            $faq         = $this->getFaqs(1, $entrynumber, $status, -1, 'datesub', 'DESC', $notNullFields);
693
            if ($faq) {
694
                $ret =& $faq[0];
695
            }
696
        }
697
698
        return $ret;
699
    }
700
701
    /**
702
     * @param  int $limit
703
     * @return array|bool
704
     */
705
    public function getContextualFaqs($limit = 0)
0 ignored issues
show
Coding Style introduced by
getContextualFaqs uses the super-global variable $_SERVER which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
706
    {
707
        $ret = false;
708
709
        $otherCriteria = new \CriteriaCompo();
710
        $otherCriteria->add(new \Criteria('modulelink', 'None', '<>'));
711
712
        $faqsObj = $this->getFaqs(0, 0, [Constants::SF_STATUS_PUBLISHED, Constants::SF_STATUS_NEW_ANSWER], -1, 'datesub', 'DESC', '', true, $otherCriteria);
713
714
        $totalfaqs  = count($faqsObj);
0 ignored issues
show
Unused Code introduced by
$totalfaqs 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...
715
        $randomFaqs = [];
716
        if ($faqsObj) {
717
            foreach ($faqsObj as $i => $iValue) {
718
                $display = false;
0 ignored issues
show
Unused Code introduced by
$display 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...
719
720
                $http        = (false === strpos(XOOPS_URL, 'https://')) ? 'http://' : 'https://';
721
                $phpself     = $_SERVER['PHP_SELF'];
722
                $httphost    = $_SERVER['HTTP_HOST'];
723
                $querystring = $_SERVER['QUERY_STRING'];
724
                if ('' != $querystring) {
725
                    $querystring = '?' . $querystring;
726
                }
727
                $currenturl     = $http . $httphost . $phpself . $querystring;
728
                $fullcontexturl = XOOPS_URL . '/' . $iValue->contextpage();
729
                switch ($iValue->modulelink()) {
730
                    case '':
731
                        $display = false;
732
                        break;
733
                    case 'None':
734
                        $display = false;
735
                        break;
736
                    case 'All':
737
                        $display = true;
738
                        break;
739
                    case 'url':
740
                        if ($iValue->exacturl()) {
741
                            $display = ($currenturl == $fullcontexturl);
742
                        } else {
743
                            $display = (false === strpos($currenturl, $fullcontexturl));
744
                        }
745
                        break;
746
                    default:
747
                        if (false === strpos($currenturl, XOOPS_URL . '/modules/')) {
748
                            $display = false;
749
                        } else {
750
                            if (false === strpos($currenturl, $iValue->modulelink())) {
751
                                $display = false;
752
                            } else {
753
                                $display = true;
754
                            }
755
                        }
756
                        break;
757
                }
758
                if ($display) {
759
                    $randomFaqs[] =& $faqsObj[$i];
760
                }
761
            }
762
        }
763
764
        if (count($randomFaqs) > $limit) {
765
            mt_srand((float)microtime() * 10000000);
766
            $rand_keys = array_rand($randomFaqs, $limit);
767
            for ($j = 0, $jMax = count($rand_keys); $j < $jMax; ++$j) {
768
                $ret[] =& $randomFaqs[$rand_keys[$j]];
769
            }
770
        } else {
771
            $ret =& $randomFaqs;
772
        }
773
774
        return $ret;
775
    }
776
777
    /**
778
     * @param  array $status
779
     * @return array
780
     */
781
    public function getLastPublishedByCat($status = [Constants::SF_STATUS_PUBLISHED, Constants::SF_STATUS_NEW_ANSWER])
782
    {
783
        $ret       = [];
784
        $faqclause = '';
785
        if (!Smartfaq\Utility::userIsAdmin()) {
786
            /** @var \XoopsModules\Smartfaq\PermissionHandler $smartPermHandler */
787
            $smartPermHandler = \XoopsModules\Smartfaq\Helper::getInstance()->getHandler('Permission');
788
            $items            = $smartPermHandler->getPermissions('item');
789
            $faqclause        = ' AND faqid IN (' . implode(',', $items) . ')';
790
        }
791
792
        $sql  = "CREATE TEMPORARY TABLE tmp (categoryid INT(8) UNSIGNED NOT NULL,datesub INT(11) DEFAULT '0' NOT NULL);";
793
        $sql2 = ' LOCK TABLES ' . $this->db->prefix('smartfaq_faq') . ' READ;';
794
        $sql3 = ' INSERT INTO tmp SELECT categoryid, MAX(datesub) FROM ' . $this->db->prefix('smartfaq_faq') . ' WHERE status IN (' . implode(',', $status) . ") $faqclause GROUP BY categoryid;";
795
        $sql4 = ' SELECT ' . $this->db->prefix('smartfaq_faq') . '.categoryid, faqid, question, uid, ' . $this->db->prefix('smartfaq_faq') . '.datesub FROM ' . $this->db->prefix('smartfaq_faq') . ', tmp
796
                              WHERE ' . $this->db->prefix('smartfaq_faq') . '.categoryid=tmp.categoryid AND ' . $this->db->prefix('smartfaq_faq') . '.datesub=tmp.datesub;';
797
        /*
0 ignored issues
show
Unused Code Comprehensibility introduced by
48% 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...
798
        //Old implementation
799
        $sql = "SELECT categoryid, faqid, question, uid, MAX(datesub) AS datesub FROM ".$this->db->prefix("smartfaq_faq")."
800
               WHERE status IN (". implode(',', $status).")";
801
        $sql .= " GROUP BY categoryid";
802
        */
803
        $this->db->queryF($sql);
804
        $this->db->queryF($sql2);
805
        $this->db->queryF($sql3);
806
        $result = $this->db->query($sql4);
807
        $error  = $this->db->error();
808
        $this->db->queryF('UNLOCK TABLES;');
809
        $this->db->queryF('DROP TABLE tmp;');
810
        if (!$result) {
811
            trigger_error('Error in getLastPublishedByCat SQL: ' . $error);
812
813
            return $ret;
814
        }
815 View Code Duplication
        while ($row = $this->db->fetchArray($result)) {
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...
816
            $faq = new Smartfaq\Faq();
817
            $faq->assignVars($row);
818
            $ret[$row['categoryid']] =& $faq;
819
            unset($faq);
820
        }
821
822
        return $ret;
823
    }
824
825
    /**
826
     * delete FAQs matching a set of conditions
827
     *
828
     * @param  object $criteria {@link CriteriaElement}
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be object|null?

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...
829
     * @return bool   FALSE if deletion failed
830
     */
831 View Code Duplication
    public function deleteAll($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...
832
    {
833
        $sql = 'DELETE FROM ' . $this->db->prefix('smartfaq_faq');
834
        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...
835
            $sql .= ' ' . $criteria->renderWhere();
836
        }
837
        if (!$this->db->query($sql)) {
838
            return false;
839
            // TODO : Also delete the permissions related to each FAQ
840
        }
841
842
        return true;
843
    }
844
845
    /**
846
     * Change a value for FAQ with a certain criteria
847
     *
848
     * @param string $fieldname  Name of the field
849
     * @param string $fieldvalue Value to write
850
     * @param object $criteria   {@link CriteriaElement}
0 ignored issues
show
Documentation introduced by
Should the type for parameter $criteria not be object|null?

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...
851
     *
852
     * @return bool
853
     **/
854 View Code Duplication
    public function updateAll($fieldname, $fieldvalue, $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...
855
    {
856
        $set_clause = is_numeric($fieldvalue) ? $fieldname . ' = ' . $fieldvalue : $fieldname . ' = ' . $this->db->quoteString($fieldvalue);
857
        $sql        = 'UPDATE ' . $this->db->prefix('smartfaq_faq') . ' SET ' . $set_clause;
858
        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...
859
            $sql .= ' ' . $criteria->renderWhere();
860
        }
861
        if (!$this->db->queryF($sql)) {
862
            return false;
863
        }
864
865
        return true;
866
    }
867
868
    /**
869
     * @param $faqid
870
     * @return bool
871
     */
872
    public function updateCounter($faqid)
873
    {
874
        $sql = 'UPDATE ' . $this->db->prefix('smartfaq_faq') . ' SET counter=counter+1 WHERE faqid = ' . $faqid;
875
        if ($this->db->queryF($sql)) {
876
            return true;
877
        } else {
878
            return false;
879
        }
880
    }
881
882
    /**
883
     * @param  string $notNullFields
884
     * @param  bool   $withAnd
885
     * @return string
886
     */
887
    public function NotNullFieldClause($notNullFields = '', $withAnd = false)
888
    {
889
        $ret = '';
890
        if ($withAnd) {
891
            $ret .= ' AND ';
892
        }
893
        if (!empty($notNullFields) && is_array($notNullFields)) {
894
            foreach ($notNullFields as $v) {
895
                $ret .= " ($v IS NOT NULL AND $v <> ' ' )";
896
            }
897
        } elseif (!empty($notNullFields)) {
898
            $ret .= " ($notNullFields IS NOT NULL AND $notNullFields <> ' ' )";
899
        }
900
901
        return $ret;
902
    }
903
904
    /**
905
     * @param  array  $queryarray
906
     * @param  string $andor
907
     * @param  int    $limit
908
     * @param  int    $offset
909
     * @param  int    $userid
910
     * @return array
911
     */
912
    public function getFaqsFromSearch($queryarray = [], $andor = 'AND', $limit = 0, $offset = 0, $userid = 0)
0 ignored issues
show
Coding Style introduced by
getFaqsFromSearch uses the super-global variable $GLOBALS which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
913
    {
914
        global $xoopsUser;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
915
916
        $ret = [];
917
918
        $userIsAdmin = Smartfaq\Utility::userIsAdmin();
919
920
        if (0 != $userid) {
921
            $criteriaUser = new \CriteriaCompo();
922
            $criteriaUser->add(new \Criteria('faq.uid', $userid), 'OR');
923
            $criteriaUser->add(new \Criteria('answer.uid', $userid), 'OR');
924
        }
925
926
        if ($queryarray) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $queryarray of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
927
            $criteriaKeywords = new \CriteriaCompo();
928
            foreach ($queryarray as $iValue) {
929
                $criteriaKeyword = new \CriteriaCompo();
930
                $criteriaKeyword->add(new \Criteria('faq.question', '%' . $iValue . '%', 'LIKE'), 'OR');
931
                $criteriaKeyword->add(new \Criteria('answer.answer', '%' . $iValue . '%', 'LIKE'), 'OR');
932
                $criteriaKeywords->add($criteriaKeyword, $andor);
933
                unset($criteriaKeyword);
934
            }
935
        }
936
937
        // Categories for which user has access
938
        if (!$userIsAdmin) {
939
            /** @var \XoopsModules\Smartfaq\PermissionHandler $smartPermHandler */
940
            $smartPermHandler = \XoopsModules\Smartfaq\Helper::getInstance()->getHandler('Permission');
941
942
            $categoriesGranted = $smartPermHandler->getPermissions('category');
943
            $faqsGranted       = $smartPermHandler->getPermissions('item');
944
            if (!$categoriesGranted) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $categoriesGranted of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
945
                return $ret;
946
            }
947
            if (!$faqsGranted) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $faqsGranted of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
948
                return $ret;
949
            }
950
            $grantedCategories = new \Criteria('faq.categoryid', '(' . implode(',', $categoriesGranted) . ')', 'IN');
951
            $grantedFaq        = new \CriteriaCompo();
952
            $grantedFaq->add(new \Criteria('faq.faqid', '(' . implode(',', $faqsGranted) . ')', 'IN'), 'OR');
953
            // If user is anonymous, check if the FAQ allow partialview
954
            if (!is_object($xoopsUser)) {
955
                $grantedFaq->add(new \Criteria('partialview', '1'), 'OR');
956
            }
957
        }
958
959
        $criteriaPermissions = new \CriteriaCompo();
960
        if (!$userIsAdmin) {
961
            $criteriaPermissions->add($grantedCategories, 'AND');
0 ignored issues
show
Bug introduced by
The variable $grantedCategories 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...
962
            $criteriaPermissions->add($grantedFaq, 'AND');
0 ignored issues
show
Bug introduced by
The variable $grantedFaq 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...
963
        }
964
965
        $criteriaAnswersStatus = new \CriteriaCompo();
966
        $criteriaAnswersStatus->add(new \Criteria('answer.status', Constants::SF_AN_STATUS_APPROVED));
967
968
        $criteriaFasStatus = new \CriteriaCompo();
969
        $criteriaFasStatus->add(new \Criteria('faq.status', Constants::SF_STATUS_OPENED), 'OR');
970
        $criteriaFasStatus->add(new \Criteria('faq.status', Constants::SF_STATUS_PUBLISHED), 'OR');
971
972
        $criteria = new \CriteriaCompo();
973
        if (!empty($criteriaUser)) {
974
            $criteria->add($criteriaUser, 'AND');
975
        }
976
977
        if (!empty($criteriaKeywords)) {
978
            $criteria->add($criteriaKeywords, 'AND');
979
        }
980
981
        if (!empty($criteriaPermissions) && (!$userIsAdmin)) {
982
            $criteria->add($criteriaPermissions);
983
        }
984
985
        if (!empty($criteriaAnswersStatus)) {
986
            $criteria->add($criteriaAnswersStatus, 'AND');
987
        }
988
989
        if (!empty($criteriaFasStatus)) {
990
            $criteria->add($criteriaFasStatus, 'AND');
991
        }
992
993
        $criteria->setLimit($limit);
994
        $criteria->setStart($offset);
995
        $criteria->setSort('faq.datesub');
996
        $criteria->setOrder('DESC');
997
998
        $sql = 'SELECT faq.faqid, faq.question, faq.datesub, faq.uid FROM ' . $this->db->prefix('smartfaq_faq') . ' AS faq INNER JOIN ' . $this->db->prefix('smartfaq_answers') . ' AS answer ON faq.faqid = answer.faqid';
999
1000
        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...
1001
            $whereClause = $criteria->renderWhere();
1002
1003
            if ('WHERE ()' !== $whereClause) {
1004
                $sql .= ' ' . $criteria->renderWhere();
1005
                if ('' != $criteria->getSort()) {
1006
                    $sql .= ' ORDER BY ' . $criteria->getSort() . ' ' . $criteria->getOrder();
1007
                }
1008
                $limit = $criteria->getLimit();
1009
                $start = $criteria->getStart();
1010
            }
1011
        }
1012
1013
        //echo "<br>" . $sql . "<br>";
0 ignored issues
show
Unused Code Comprehensibility introduced by
42% 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...
1014
1015
        $result = $this->db->query($sql, $limit, $start);
0 ignored issues
show
Bug introduced by
The variable $start 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...
1016
        if (!$result) {
1017
            trigger_error('Query did not work in smartfaq', E_USER_WARNING);
1018
1019
            return $ret;
1020
        }
1021
1022
        if (0 == $GLOBALS['xoopsDB']->getRowsNum($result)) {
1023
            return $ret;
1024
        }
1025
1026 View Code Duplication
        while (false !== ($myrow = $this->db->fetchArray($result))) {
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...
1027
            $faq = new Smartfaq\Faq();
1028
            $faq->assignVars($myrow);
1029
            $ret[] =& $faq;
1030
            unset($faq);
1031
        }
1032
1033
        return $ret;
1034
    }
1035
1036
    /**
1037
     * @param  int   $cat_id
1038
     * @param        $status
1039
     * @return array
1040
     */
1041
    public function getCountsByCat($cat_id = 0, $status)
1042
    {
1043
        global $xoopsUser;
0 ignored issues
show
Compatibility Best Practice introduced by
Use of global functionality is not recommended; it makes your code harder to test, and less reusable.

Instead of relying on global state, we recommend one of these alternatives:

1. Pass all data via parameters

function myFunction($a, $b) {
    // Do something
}

2. Create a class that maintains your state

class MyClass {
    private $a;
    private $b;

    public function __construct($a, $b) {
        $this->a = $a;
        $this->b = $b;
    }

    public function myFunction() {
        // Do something
    }
}
Loading history...
1044
        $ret = [];
1045
        $sql = 'SELECT categoryid, COUNT(*) AS count FROM ' . $this->db->prefix('smartfaq_faq');
1046
        if ((int)$cat_id > 0) {
1047
            $sql .= ' WHERE categoryid = ' . (int)$cat_id;
1048
            $sql .= ' AND status IN (' . implode(',', $status) . ')';
1049
        } else {
1050
            $sql .= ' WHERE status IN (' . implode(',', $status) . ')';
1051
            if (!Smartfaq\Utility::userIsAdmin()) {
1052
                /** @var \XoopsModules\Smartfaq\PermissionHandler $smartPermHandler */
1053
                $smartPermHandler = \XoopsModules\Smartfaq\Helper::getInstance()->getHandler('Permission');
1054
                $items            = $smartPermHandler->getPermissions('item');
1055
                if (is_object($xoopsUser)) {
1056
                    $sql .= ' AND faqid IN (' . implode(',', $items) . ')';
1057
                } else {
1058
                    $sql .= ' AND (faqid IN (' . implode(',', $items) . ') OR partialview = 1)';
1059
                }
1060
            }
1061
        }
1062
        $sql .= ' GROUP BY categoryid';
1063
1064
        //echo "<br>" . $sql . "<br>";
0 ignored issues
show
Unused Code Comprehensibility introduced by
42% 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...
1065
1066
        $result = $this->db->query($sql);
1067
        if (!$result) {
1068
            return $ret;
1069
        }
1070
        while ($row = $this->db->fetchArray($result)) {
1071
            $ret[$row['categoryid']] = (int)$row['count'];
1072
        }
1073
1074
        return $ret;
1075
    }
1076
}
1077