1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23
function create_link($string) {
$input = media_title($string);
$input = html_entity_decode($input, ENT_QUOTES);
$input = clearDiacritics($input);
$input = strtolower($input);
$input = preg_replace('/_$/i', '', $input);
$input = preg_replace('/^_/i', '', $input);
$input = trim(ereg_replace(' +', ' ', preg_replace('/[^a-z0-9\s]/', '', $input)));
$input = str_replace(' ', '-', $input);
$input = preg_replace('/[^0-9a-z\-]+/', '', $input);
$input = preg_replace('/[\-]+/', '-', $input);
$input = preg_replace('/_[_]*/i', '-', $input);
$input = trim($input, '-');
return $input;
}
Refactorings
No refactoring yet !
Nick Stinemates
September 4, 2008, September 04, 2008 18:11, permalink
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
function create_link($string) {
return sanitize_input(strtolower(clearDiacritics(html_entity_decode(media_title($string), ENT_QUOTES))));
}
function sanitize_input($input) {
// this is now reusable.
$input = preg_replace('/_$/i', '', $input);
$input = preg_replace('/^_/i', '', $input);
$input = trim(ereg_replace(' +', ' ', preg_replace('/[^a-z0-9\s]/', '', $input)));
$input = str_replace(' ', '-', $input);
$input = preg_replace('/[^0-9a-z\-]+/', '', $input);
$input = preg_replace('/[\-]+/', '-', $input);
$input = preg_replace('/_[_]*/i', '-', $input);
$input = trim($input, '-');
return $input;
}
Scott Reynen
September 4, 2008, September 04, 2008 19:20, permalink
What a mess of redundant code. It looks like it was designed specifically as a refactoring test. Here's what I changed:
1) you're doing replaces on spaces, converting spaces to dashes, and then doing the exact same replaces on dashes
2a) You're trimming underscores, converting underscores to dashes, and then trimming on dashes
2b) And you're doing all of this after you've already removed all the underscores
3) There's no reason to do case-insensitive replaces when you've already converted everything to lowercase
4) You can consolidate multiple preg_replace calls into one with arrays
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
function create_link($string) {
$input = media_title($string);
$input = html_entity_decode($input, ENT_QUOTES);
$input = clearDiacritics($input);
$input = strtolower($input);
$input = str_replace(' ', '-', $input);
$input = preg_replace(
array( '/[\-]+/' , '/[^0-9a-z\-]+/' ) ,
array( '-' , '' ) ,
$input
);
$input = trim( $input , '-' );
return $input;
}
Please help shorten the code, so that he was still the same function.
It just looks unprofessional! ;)
media_title() && clearDiacritics() - These functions are needed, so please do not eliminate