Jump to content

Recommended Posts

Hi all,

 

I have a question about this function.

 

What should result of following code be?

 

$test = (int)0;

tep_not_null($tst);

 

I get 'false', but I would think value of '0' is a valid one and result should be 'true';

 

Looking at the code (includes/functions/general.php), on line 1140, code is:

 if (($value != '') && (strtolower($value) != 'null') && (strlen(trim($value)) > 0))

 

I think first condition is incorrect. Instead of ($value != ''), it should be ($value !== ''). Then value of 0 is being processed correctly.

 

Any thoughts?

 

Thanks,

Rudolf

Share this post


Link to post
Share on other sites

The function is for checking a string to see if it is empty. $text in your example isn't a string. For it you would just use $text > 0.

Share this post


Link to post
Share on other sites

Hi,

 

I agree, this is what function should be used.

However, this is not the case -- it is being used to check integer values. I found this while debugging shipping module.

In includes/classes/shipping.php. Function cheapest() (Line 98),

if (isset($quotes['methods'][$i]['cost']) && tep_not_null($quotes['methods'][$i]['cost']))

 

Cost is checked with this function.

 

Name of function is misleading, as it implies value is checked for being a "null" (not set) and there is no indication (or checks for that matter) that function should only be used with strings.

 

Rudolf

Share this post


Link to post
Share on other sites

The modules are returning a string. The code is checking that the string is not empty. The values of the string are then compared to get the lowest number.

 

Are you just asking out of curiosity or is the shipping not working correctly on your site? If the latter, you should post the problem.

Share this post


Link to post
Share on other sites

Handling of nulls, undefineds, empty/zero, strings vs. numbers, and other boundary conditions varies from language to language, and can be quite difficult to grok (especially if this behavior has never been carefully and rigidly defined!). I would suggest a substantial amount of Googling before declaring that some code is in error, just because it looks "wrong" to you. On the other hand, if the code doesn't seem to work for certain cases, and your proposed fix works for those (and doesn't break any currently working cases), you may have something there.

Share this post


Link to post
Share on other sites

One thing is you have your variable as $test but you're checking $tst. 


I'm not really a dog.

Share this post


Link to post
Share on other sites

One thing is you have your variable as $test but you're checking $tst. 

 

Yes, of course. Me and my fingers.. But you got the idea.

 

 

Handling of nulls, undefineds, empty/zero, strings vs. numbers, and other boundary conditions varies from language to language, and can be quite difficult to grok (especially if this behavior has never been carefully and rigidly defined!). I would suggest a substantial amount of Googling before declaring that some code is in error, just because it looks "wrong" to you. On the other hand, if the code doesn't seem to work for certain cases, and your proposed fix works for those (and doesn't break any currently working cases), you may have something there.

 

This is why I posted it in dev forum, not reported a problem. I was not sure if behavior I see is correct or not. My change does not break the existing code (as far as I tested), but does handle integer '0' correctly. Whether this function should or should not be used for integer values is a different matter. I did not see any documentation that suggests use of it and function certainly accepts integer values happily.

I came from C background and having loose types in PHP certainly gets on my nerves :-)

 

Rudolf

Share this post


Link to post
Share on other sites

The OP is right as I've spent most of the afternoon trying to figure out why my shipping module didin't work. The shipping class function cheapest uses this:

if (isset($quotes['methods'][$i]['cost']) && tep_not_null($quotes['methods'][$i]['cost'])) {
                $rates[] = array('id' => $quotes['id'] . '_' . $quotes['methods'][$i]['id'],
                                 'title' => $quotes['module'] . ' (' . $quotes['methods'][$i]['title'] . ')',
                                 'cost' => $quotes['methods'][$i]['cost']);
              }

And since my shipping module returns a zero value for 'cost'- eg Free Shipping, the tep_not_null was giving false. It wasn't until I checked how tep_not_null worked that I agree in the post above that it used for strings (or blank arrays), but in this case it is being used on a numeric value. So either this class is wrong, or the tep_not_null needs a revision.

If anybody had the same issue as me then just to change the if to
    

        if( $quotes['methods'][$i]['cost'] >= 0 ) {
                $rates[] = array('id' => $quotes['id'] . '_' . $quotes['methods'][$i]['id'],
                                 'title' => $quotes['module'] . ' (' . $quotes['methods'][$i]['title'] . ')',
                                 'cost' => $quotes['methods'][$i]['cost']);
                                 
            }

 


t o n e u s

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×