There are several things to point out here.
For the record, I rewrote and tested the process without a class on my localhost -- the adjustment for you to convert my snippet to suit your class should be simple.
New Snippet:
$slugs = <<<SLUGS
AMERIS BANCORP
ALCENTRA CAPITAL CORP
ABEONA THERAPEUTICS INC
SLUGS; // yes, I tested the entire battery
const COMPANY_STOPWORDS = '11000th|american|and|beneficial|bond|class|common|company|corporation|corp|commodity|cumulative|co|daily|dep|depositary|depository|debentures|diversified|due|each|etf|equal|equity|exchange|financial|fund|fixedtofloating|fixed|floating|group|healthcare|holdings|holding|inc|incorporated|interests|interest|in|index|income|junior|liability|limited|lp|llc|ltd|long|markets|maturity|municipal|muni|monthly|noncumulative|notes|no|of|one|or|portfolio|pay|partnership|partner|par|perpetual|per|perp|pfd|preference|preferred|redeemable|repstg|representing|represents|rate|sa|smallcap|series|shs|shares|share|short|stock|subordinated|ser|senior|the|three|term|to|traded|trust|two|ultrashort|ultra|value|warrant|weight|[a-z]';
function slugCompany($slug){
return trim(preg_replace(['/[^\da-z\s]+|\b(?:' . COMPANY_STOPWORDS . ')\b/', '/\s+/'], ['', '-'], strtolower($slug)), '-');
}
foreach (explode(PHP_EOL, $slugs) as $slug) {
echo "<div>$slug => " , slugCompany($slug) , "</div>";
}
Output:
AMERIS BANCORP => ameris-bancorp
ALCENTRA CAPITAL CORP => alcentra-capital
ABEONA THERAPEUTICS INC => abeona-therapeutics
First about your code...
public static function slugCompany($c){
$c=strtolower($c);
$c=preg_replace('/[^\da-z\s]/i', '', $c);
$words=self::COMPANY_STOPWORDS;
$c=preg_replace('/\b('.$words.')\b/i', '', $c);
$c=preg_replace('/(\s+)/i', '-', trim($c));
return $c;
}
- You are making three separate passes through the string(s).
- You are inefficiently finding-replacing each individual "non-digit/non-alpha/non-space" character.
- You are making case-insensitive matching, but you know everything is already lowercase.
- The capture group on the third
preg_replace() call is unnecessary.
- You should use one space on either side of the
= assignment operator.
Regarding ArtisticPhoenix's post, it makes a single preg_replace() call and places some importance on readability, but...
public static function slugCompany($company){
$replace = [
'/[^\da-z\s]/i' => '', //remove punctuation I guess
'/\b('.self::COMPANY_STOPWORDS.')\b/i' => '', //remove these companies
'/^\s+|\s+$/' => '', //trim
'/\s+/' => '-' //replace space with -
];
return preg_replace(array_keys($replace), $replace, strtolower($company));
}
preg_replace() is making too many separate passes through the string(s).- Case-insensitive matching is used, but everything is already lowercase.
Some basic principles:
- Avoid using regular expressions if there is a single non-regex function that can do the same job in a sensible/readable manner. In my snippet, I am using
trim(..., '-') to trim the leading/trailing hyphens rather than another replacement pass of ~^-+|-+$~.
- Endeavor to reduce total function calls where it doesn't negatively impact your script logic. In this case, there is no use in calling
preg_replace() multiple times because the function is happy to receive an array of patterns and an array of replacements.
- When multiple patterns share a duplicate replacement string, try to merge the two patterns -- this means the task is completed in one pass.
- When the intention is to matching multiple characters that may possibly be side-by-side, match as many as possible. Consider this scenario, you are standing over an open carton of eggs. I ask you to pick up all the eggs. Doing 12 squats to extract each egg individually (while good for personal health) is an obviously inefficient choice. Just reach down one time and pick up the whole carton -- done in one pass.
- Don't use capture groups unless you actually need to capture substrings. While this extends the pattern length by 2 characters, it is a clear indication of pattern intention and removes the possibility of numbered backreferences associated with capture groups. In other words, use
(?:...) versus (...).
- Avoid declaring single-use variables. There are a few exceptions to this rule (like adding clarity to a line of code via a descriptive variable name AND to reduce horizontal scrolling in a script for devs), but in this case I felt returning a one-liner was tolerable.
- You can condense 26 of the alternatives in your
const by writing [a-z] versus individually listing letters. This will improve pattern efficiency and pattern length.
- If pattern efficient is the highest priority AND the piped
const values are very rarely modified, you could go the extra mile and try to reduce the number of "alternatives". For instance, you can combine ...|of|or|o|... to be ...|o[fr]?|... and so on. Some would say that this comes at a cost of readability/maintainability.
