Can I do This with empty Check

[mickjav]

The code below is the original

Code Select Expand

        if (!empty($_REQUEST['cat']))
            $cat = (int) $_REQUEST['cat'];
        else
            $cat = 0;

        if (empty($cat))
            fatal_error($txt['smflinks_nocatselected'], false);
Can somebody let me know if there is a reason why I can't do the below with the statment.

Code Select Expand

        if (!empty($_REQUEST['cat']))
            $cat = (int) $_REQUEST['cat'];
        else
            fatal_error($txt['smflinks_nocatselected'], false);
All the best mick

[Aleksi "Lex" Kilpinen]

Really not sure about this, but it looks like the original code tries to make sure that $cat is defined, if empty, maybe to avoid an undefined index error later?

[mickjav]

Sorry didn't think I needed to add it but there is also A isset

Code Select Expand

if (isset($_REQUEST['cat']))
{
if (!empty($_REQUEST['cat']))
$cat = (int) $_REQUEST['cat'];
else
$cat = 0;

if (empty($cat))
fatal_error($txt['smflinks_nocatselected'], false);

[Arantor]

The reason? What happens if someone tries index.php?cat=sometexthere

In the first piece of code, $_REQUEST['cat'] is defined, $cat is set to (int) of that string, which is zero. So when you hit the secondary check, you have weeded out not only that cat is not zero, but that it is also not a non-integer.

In the second piece of code, if fed some text, you will get $cat containing 0 but still passing your test, which will then fail further down the line in a way that you don't want.

Trying to shortcut code is usually a bad idea unless you fully understand what every possible branch can do and whether any possible branch will be triggered by conditions you don't expect.

There is also no actual benefit to keeping the code as lean as you're trying to there, it is not a performance benefit.

[mickjav]

Thanks @Arantor Just trying to understand how things work

I have tested the updated code below

With changing the Address to Cat=0 and cat=test Both give me the message

Code Select Expand

if (isset($_REQUEST['cat']))
{
if (!empty($_REQUEST['cat']))
$cat = (int) $_REQUEST['cat'];
else
fatal_error($txt['smflinks_nocatselected'], false);
But will return it to the original

Many thanks for your help mick

[Arantor]

Then it's something else catching it - that code as given will not filter out $cat=test, but I suspect when it goes to look up $cat later on against the relevant table, 0 won't match any rows. But it's still getting further than it should.

[mickjav]

cool thanks marking solved

Edit Found second check after query, which is why I'm getting same message

Code Select Expand

if (empty($row['ID_CAT']))
fatal_error($txt['smflinks_nocatselected'], false);

[Arantor]

Yes, you get the same overall outcome - but the extra check before the query means you save the query, which is definitely a performance saving.