patternphpMinor
String helper class
Viewed 0 times
helperclassstring
Problem
I have class with some basic string operations. Any suggestion to improve the class
class StringHelpers
{
function replace_between($str, $needle_start, $needle_end, $replacement) {
$pos = strpos($str, $needle_start);
$start = $pos === false ? 0 : $pos + strlen($needle_start);
$pos = strpos($str, $needle_end, $start);
$end = $start === false ? strlen($str) : $pos;
return substr_replace($str,$replacement, $start, $end - $start);
}
function getTextBetweenTags($string, $tagname) {
$pattern = "/([\w\W]*?)/";
preg_match($pattern, $string, $matches);
return $matches[1];
}
function checkIfValueContainLetters($value){
return preg_match('/[A-Z]+[a-z]+[0-9]+/', $value);
}
public function createTableRowElementsByValue($bgColor, $tdName, $tdValue)
{
$tableRow = '' . $tdName . '' . $tdValue . ' ';
return $tableRow;
}
public function changeBgColorWhiteToBlue()
{
if ($this->color == 'D4E4F3') {
$this->color = '';
return '';
}
$this->color = 'D4E4F3';
return 'D4E4F3';
}
public function createTableRow($tdName)
{
return ''. $tdName.'';
}
}Solution
These functions do widely different things (replace stuff in a generic string, handle HTML, handle colors, ...), so there are probably better places for them then a generic
StringHelpers class. A class like this would grow quite large if you handle all functions like this. createTableRowElementsByValue for example doesn't really have anything to do with strings, and changeBgColorWhiteToBlue is something else entirely.- Naming:
checkIfValueContainLettersis very misleading. I'm not sure if the regex is even correct, since it checks for such an odd thing (definitely a thing too specific for a genericStringHelpersclass). Did you maybe mean^[A-Za-z0-9]*$? Because that would somewhat match your name (except for the numbers). If that's what you want,isAlphaNumericwould be a better name.
- Naming:
changeBgColorWhiteToBluealso doesn't do what it's name suggest. It sets the color toD4E4F3, except if it is alreadyD4E4F3, in which case it sets it to nothing; there is no white involved.
- Naming:
createTableRowdoesn't create a row (that would betr).
getTextBetweenTagsisn't very stable. Input such asba/&!r,foo, ... would break it.
Context
StackExchange Code Review Q#116705, answer score: 3
Revisions (0)
No revisions yet.