HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

String helper class

Submitted by: @import:stackexchange-codereview··
0
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: 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 ba/&!r, foo, ... would break it.

Context

StackExchange Code Review Q#116705, answer score: 3

Revisions (0)

No revisions yet.