CODEWITHSUNDEEP
php
Timido
Timido

Reputation: 1776

PHP: refactoring count() to avoid warning "Parameter must be an array or an object that implements Countable"

I have a code that implements a stemming algorithm. One of the routines contain many lines similar to this one:

if(count($ret_str = $this->delete_suff(REDACTED LIST OF ARGUMENTS))) return $ret_str;

The delete_suff() function simply returns a string (which might be the empty string), or it may return null (see code below).

Running the code on PHP 7.4 I get the warning "Parameter must be an array or an object that implements Countable", which I would like to get rid of.

On the basis of my understanding, this is what the original code does:

What is the correct way of refactoring this code to avoid such incorrect use of count()?

On a first try I would remove the count() altogether:

if($ret_str = $this->delete_suff(REDACTED LIST OF ARGUMENTS)) return $ret_str;

But I guess that this way the if condition would return false when $ret_str is empty - which would not be the original intended behavior since count('') = 1.

A second option I came up with is:

if(!is_null($ret_str = $this->delete_suff(REDACTED LIST OF ARGUMENTS))) return $ret_str;

Which one is correct? Or, if none is correct, what would be?

EDIT: for completeness sake, this is the function that is called:

private function delete_suff($arr_suff,$str,$str_len,$where,$ovunque=false) {
        if($where==='r2') $r = $this->return_R2($str);
        else if($where==='rv') $r = $this->return_RV($str);
        else if($where==='r1') $r = $this->return_R1($str);
        
        $r_len = mb_strlen($r);
        
        if($ovunque) {
            foreach($arr_suff as $suff) {
                if($str_len-mb_strlen($suff) < 0) continue;
                $pos = mb_strpos($str,$suff,$str_len-mb_strlen($suff));
                if($pos !== false) {
                    $pattern = '/'.$suff.'$/u';
                    $ret_str = preg_match($pattern,$r) ? mb_substr($str,0,$pos) : '';
                    if($ret_str !== '') return $ret_str;
                    break;
                }
            }
        }
        else {
            foreach($arr_suff as $suff) {
                if($r_len-mb_strlen($suff) < 0) continue;
                $pos = mb_strpos($r,$suff,$r_len-mb_strlen($suff));
                if($pos !== false) return mb_substr($str,0,$pos+$str_len-$r_len);
            }
        }
    }

Upvotes: 0

Views: 135

Answers (1)

u_mulder
u_mulder

Reputation: 54796

To refactor your code you should understand what kind of values you want to filter with if clause.

As you say in comments - you need only strings, including empty string, so this can be stated as

if (is_string($ret_str = $this->delete_suff(REDACTED LIST OF ARGUMENTS))) return $ret_str;

Update: in provided function body I see two returns:

return preg_match($pattern,$r) ? mb_substr($str,0,$pos) : '';
return mb_substr($str,0,$pos+$str_len-$r_len);

According to manual, mb_substr returns string.

Also there's one implicit return null, when if conditions are not met.

So, delete_suff returns either string or null.

That's why you can use either is_string() (cause null is not string) or use strict comparison with null:

if (null !== (ret_str = $this->delete_suff(REDACTED LIST OF ARGUMENTS))) return $ret_str;

Which ine to use - up to you to decide. But I would use is_string as it clearly identifies what data you need and what data should be skipped.

Upvotes: 1

Related Questions