String helper class

Question

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 = "/<$tagname>([\w\W]*?)<\/$tagname>/";
          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 = '<tr bgcolor="' . $bgColor . '"><td>' . $tdName . '</td><td>' . $tdValue . '</td>  </tr>';
        return $tableRow;
    }

    public function changeBgColorWhiteToBlue()
    {
        if ($this->color == 'D4E4F3') {
            $this->color = '';
            return '';
        }
        $this->color = 'D4E4F3';
        return 'D4E4F3';
    }

    public function createTableRow($tdName)
    {
        return '<td>'. $tdName.'</td>';
    }

    }

Answer 1

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: checkIfValueContainLetters is 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 generic StringHelpers class). 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, isAlphaNumeric would be a better name.
• Naming: changeBgColorWhiteToBlue also doesn't do what it's name suggest. It sets the color to D4E4F3, except if it is already D4E4F3, in which case it sets it to nothing; there is no white involved.
• Naming: createTableRow doesn't create a row (that would be tr).
• getTextBetweenTags isn't very stable. Input such as <foo>ba/&!r</foo>, foo, ... would break it.

Answer 2

I haven't code PHP in a while, but some thing can be applied to multiple languages:

1) parameters checking - replace_between function does not check input parameters. While providing wrong parameter (negative indexes, stop after start etc.) is generating proper exceptions in languages like C# and Java, in PHP may lead to confusing results, as explained here when using negative indexes.

2) (somewhat) ambiguous function name - getTextBetweenTags can be named getFirstTagText, as it seems to get only text found on first provided $tagname. Also, changeBgColorWhiteToBlue is doing something else (already specified in previous answer)

3) string formatting - PHP allows interpolated variables (or interpolated strings as C# calls it), but it seems to be a little slower. So,

'<tr bgcolor="' . $bgColor . '"><td>' . $tdName . '</td><td>' . $tdValue . '</td>  </tr>';

becomes something like

'<tr bgcolor='$bgColor'><td>'$tdName'</td><td>'$tdValue'</td>  </tr>';

Personally, I prefer a (very) little speed downgrade over (much) improved readability.

4) avoid magic numbers/strings - D4E4F3 appears three times in your code. I would define a constant for it. Or even better, define css styles and set/unset them.

5) class name - StringHelper suggests generic string manipulations (like replacing, padding etc.), so only replace_between function fits here. A better container name for the rest of the functions would be HtmlHelpers perhaps.

Slightly off-topic, but might be useful: if you need to parse Html content, I advice to take a look into PHP Simple HTML DOM Parser.