1
\$\begingroup\$

I developed the function array_includes which checks if an array contains a value and returns it. I would like to know if there is already such a function which is more efficient or how I can make the function more efficient.

<?php
$params = "colorgroup-b;test;abc";
$paramsArr = explode(";", $params);

var_dump(array_includes("colorgroup", $paramsArr));  //outputs: colorgroup-b

function array_includes($needle, $arr)
{       
    foreach($arr as $param) {
        if (strpos($param, $needle) !== false) {
            return $param;
        }
    }
    return false;
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ @Dave it sounds like the title but absolutely not like the code we are supposed to review here. \$\endgroup\$ – Your Common Sense Jul 5 '19 at 11:00
  • \$\begingroup\$ I would rather question this function's existence. What it supposed to return for the array like this "colorgroup-a;colorgroup-b;test;abc" and why? \$\endgroup\$ – Your Common Sense Jul 5 '19 at 11:04
  • \$\begingroup\$ ↑ The first result. But good hint. The function can be improved to return an array if there are multiple finds. But I would rather create a second function e.g. array_includes_multiple or something similar \$\endgroup\$ – Black Jul 5 '19 at 12:46
2
\$\begingroup\$

Your function checks if an array contains a string that contains your given $needle. No, there's no such function in PHP yet. The in_array() function does something else.

You could make your function more efficient if you include the explode() inside the function, like this:

function find_parameters($needle, $parameterStr)
{       
    if (strpos($parameterStr, $needle) !== false) {
        $result = [];
        foreach(explode(";", $parameterStr) as $parameter) {
            if (strpos($parameter, $needle) !== false) {
                $result[] = $parameter;
            }
        }
        return $result;
    } 
    return false;
}

This function first checks whether the parameter string actually does contain the needle. It can quickly return false when it doesn't. If the needle is present it does what your code does, with the exception that it returns an array. This is done so that it can return multiple results. Suppose your parameter string looks like this:

$params = "colorgroup-a;colorgroup-b;test;abc";

and you test this:

var_dump(find_parameters("colorgroup", $paramsArr));

Then it will not return only colorgroup-a, like your function, but it will return an array with colorgroup-a and colorgroup-b.

So this new function is more efficient, when there are no matching parameters, and it is more correct in that it finds all the parameters.

Yes, it is longer, but shortness of code should not be the main aim when writing code. The use of regular expressions is also very popular for these kind of tasks, and it would result in less code. But the engine behind regular expressions is complex, and would make the code a lot less efficient.

\$\endgroup\$
0
1
\$\begingroup\$

After a bit more consideration and soul searching, I suppose the correct technique to use would be the one that avoids an iterative "guess and check" technique and avoids changing the input's data type.

The input data type is a string and the output data type is a string; I don't find it very compelling to convert the input temporarily to an array just to return a string. If the input data began as an array, I would probably be more in favor of array type manipulation.

If you don't want a concise regex technique, just whittle down $param until it is what you need and don't generate data that you won't eventually use.

Code: (Demo)

$params = "test;colorgroup-b;test;abc";

var_dump(string_includes("group", $params));  //outputs: colorgroup-b

function string_includes($needle, $haystack) {
    $needlePosition = strpos($haystack, $needle);
    if ($needlePosition === false) {
        return false;
    }
    $needleEndPosition = strpos($haystack, ';', $needlePosition);
    if ($needleEndPosition !== false) {
        $haystack = substr($haystack, 0, $needleEndPosition);
    }
    $delimiterPosition = strrpos($haystack, ';');
    if ($delimiterPosition === false) {
        return $haystack;
    }
    return substr($haystack, $delimiterPosition + 1);
}

This snippet will call 1 to 5 string function calls no matter how big the input string. This makes the processing time efficient and relatively consistent.

This is much more code bloat than I would welcome into one of my projects, but if you are hellbent on prioritizing efficiency, this should be on your radar to benchmark.

\$\endgroup\$
1
\$\begingroup\$

In case you'd like to compare/benchmark the regex technique, it will look like this:

Code: (Demo)

$params = "colorgroup-b;test;abc";

var_dump(array_includes("colorgroup", $params));  //outputs: colorgroup-b

function array_includes($needle, $params) {
    return preg_match('/[^;]*' . preg_quote($needle, '/') . '[^;]*/', $params, $m) ? $m[0] : false;    
}

For small scale execution, no user is going to notice any performance differences.

For large scale executions, there may actually be some advantage in this way because a full length array doesn't need to be generated to begin the searching. Notice that there will potentially be n calls of strpos().

There is also the versatility of this script. It doesn't take much effort to modify the script to extract multiple matches. Or use case-insensitivity. Or multibyte awareness. Or employ word boundaries (something that non-regex techniques struggle to do simply). These adjustments are so slight that if you wanted individual command over all of the above options, you wouldn't need a separate method for each - just pass additional parameters to the method.

function stringSearch($needle, $params, $singleMatch = true, $caseInsensitive = false, $multibyte = false, $wordBoundaries = 'neither') {

(I am thinking wordBoundaries might be neither, left, right, or both.)

Returning an array is as easy as adding four-characters to preg_match to become preg_match_all. Demo

This might also be a timely opportunity to bring up Premature Optimization. Assuming the input string in your application is expected to be a length of tens-of-characters and it is not called thousands of times in an iterative process, the processing time from KIKO's snippet versus my snippet will be entirely unnoticeable.

With that criteria mostly written off as moot, other criteria can be mentioned.

If the idea of regular expressions sends shivers down the spine of you or your development team, that is a perfectly valid justification to use KIKO's technique.

If your code base already contains simple regular expression techniques like my snippet, then my snippet will conform nicely to the project.

I personally never shy away from the convenience that regex affords. I also value the directness and conciseness whenever performance is not noticeably impacted. If a non-regex technique would provide a more brief, direct process, I would opt for that. (Just my opinion of course)

p.s. if your $needle value is already sufficiently sanitized, the preg_quote call may be omittable.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.