4
\$\begingroup\$

I'm starting to learn OOP with PHP, and all I've learned so far is just by searching and reading. So I have this need to check input data for certain things like min of chars, max of chars, spaced or not spaced, just letters or not.

So far I've just created the alpha() method which is just for letters only. I'm pretty sure this is horrible coding and can and should be improved. I would love some feedback on how I can improve this class.

I also have a function that strips tags when '<' and '>' becomes available on the strings, but it's not currently in the code.

class dataValidator {

    public function alpha($data, $space = null, $minimum = null, $maximum = null, $extends = null) {

        $data = trim($data);

        if ( !empty($data) ) {

            $data = preg_replace("/\s{2,}/", " ", $data);

            if ( isset($space) && isset($minimum) && isset($maximum) && isset($extends) ) {

                if ( $minimum == 0 || $maximum < $minimum ) { return false; }

                if ( $space === true ) {

                    if ( $extends === "EXT_PUNCTUATION" ) {

                        $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                        $newData = preg_replace("/[^\p{L}\p{P}\ ]/u", "", $newData);

                        $dataLen = strlen($newData);

                        if ( $dataLen >= $minimum && $dataLen <= $maximum ) {

                            $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                            return $newData;

                        } else { return false; }

                    } else if ( $extends === "EXT_ANY" ) {

                        $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                        $newData = preg_replace("/[^\p{L}\p{P}\p{S}\ ]/u", "", $newData);

                        $dataLen = strlen($newData);

                        if ( $dataLen >= $minimum && $dataLen <= $maximum ) {

                            $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                            return $newData;

                        } else { return false; }

                    } else { return false; }

                } else {

                    $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                    $newData = preg_replace("/[^\p{L}]/u", "", $newData);

                    $dataLen = strlen($newData);

                    if ( $dataLen >= $minimum && $dataLen <= $maximum ) {

                        $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                        return $newData;

                    } else { return false; }

                }

            } else if ( isset($space) && isset($minimum) && isset($maximum) ) {

                if ( $minimum == 0 || $maximum < $minimum ) { return false; }

                if ( $space === true ) {

                    $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                    $newData = preg_replace("/[^\p{L}\ ]/u", "", $newData);

                    $dataLen = strlen($newData);

                    if ( $dataLen >= $minimum && $dataLen <= $maximum ) {

                        $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                        return $newData;

                    } else { return false; }

                } else {

                    $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                    $newData = preg_replace("/[^\p{L}]/u", "", $newData);

                    $dataLen = strlen($newData);

                    if ( $dataLen >= $minimum && $dataLen <= $maximum ) {

                        $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                        return $newData;

                    } else { return false; }

                }

            } else if ( isset($space) && isset($minimum) ) {

                if ( $minimum == 0 ) { return false; }

                if ( $space === true ) {

                    $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                    $newData = preg_replace("/[^\p{L}\ ]/u", "", $newData);

                    if ( strlen($newData) >= $minimum ) {

                        $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                        return $newData;

                    } else { return false; }

                } else {

                    $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                    $newData = preg_replace("/[^\p{L}]/u", "", $newData);

                    if ( strlen($newData) >= $minimum ) {

                        $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                        return $newData;

                    } else { return false; }

                }

            } else if ( isset($space) ) {

                if ( $space === true ) {

                    $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                    $newData = preg_replace("/[^\p{L}\ ]/u", "", $newData);
                    $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                    return $newData;

                } else { return self::alpha($data); }

            } else {

                $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, "UTF-8");
                $newData = preg_replace("/[^\p{L}]/u", "", $newData);
                $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, "UTF-8");

                return $newData;

            }

        } else { return false; }

    }

}

So using this class the following code:

$string = "Hola mis 100 canarios, un dia hermoso en la cabaña.! And what about thís guy right here.? Are this &lt;i&gt; and &lt;/i&gt; tags ?";

echo "<pre>String: $string</pre><br />";

$_validator = new dataValidator();

$newString_1 = $_validator->alpha($string);
$newString_2 = $_validator->alpha($string, true);
$newString_3 = $_validator->alpha($string, true, 500);
$newString_4 = $_validator->alpha($string, true, 20, 150);
$newString_5 = $_validator->alpha($string, true, 20, 150, "EXT_PUNCTUATION");
$newString_6 = $_validator->alpha($string, true, 20, 150, "EXT_ANY");

echo "<pre>Alpha(data) validator: $newString_1</pre>";
echo "<pre>Alpha(data, space: true) validator: $newString_2</pre>";
echo "<pre>Alpha(data, space: true, min: 500) validator: $newString_3</pre>";
echo "<pre>Alpha(data, space: true, min: 20, max: 100) validator: $newString_4</pre>";
echo '<pre>Alpha(data, space: true, min: 20, max: 100, extends: "EXT_PUNCTUATION") validator: '.$newString_5.'</pre>';
echo '<pre>Alpha(data, space: true, min: 20, max: 100, extends: "EXT_ANY") validator: '.$newString_6.'</pre>';

Will output something like:

String: Hola mis 100 canarios, un dia hermoso en la cabaña.! And what about thís guy right here.? Are this <i> and </i> tags ?
Alpha(data) validator: HolamiscanariosundiahermosoenlacabañaAndwhataboutthísguyrighthereArethisianditags
Alpha(data, space: true) validator: Hola mis  canarios un dia hermoso en la cabaña And what about thís guy right here  Are this i and i tags 
Alpha(data, space: true, min: 500) validator: 
Alpha(data, space: true, min: 20, max: 100) validator: Hola mis  canarios un dia hermoso en la cabaña And what about thís guy right here  Are this i and i tags 
Alpha(data, space: true, min: 20, max: 100, extends: "EXT_PUNCTUATION") validator: Hola mis  canarios, un dia hermoso en la cabaña.! And what about thís guy right here ? Are this i and /i tags ?
Alpha(data, space: true, min: 20, max: 100, extends: "EXT_ANY") validator: Hola mis  canarios, un dia hermoso en la cabaña.! And what about thís guy right here ? Are this <i> and </i> tags ?

For strings in Spanish, I need to check for UTF-8 characters like 'ñ, é, í' etc.

\$\endgroup\$
1
\$\begingroup\$

Standards

It's more common to start class names with a capital letter:

class DataValidator {

    public function alpha($data, $space = null, $minimum = null, $maximum = null, $extends = null) {
        $data = trim($data);

Early Return

You write (with a bunch of code in the middle):

   if ( !empty($data) ) {
   } else { return false; }

It's often easier to just write

    if ( empty($data) ) {
        return false;
    }

That way you can see immediately what happens when $data is empty, and you can see that the rest of the function is about when it isn't. This only works when you return to end the else, but that's a relatively common pattern.

Note that I also moved the single statement onto its own line. This makes it more consistent with the multiline then blocks.

Perl Regular Expressions

       $data = preg_replace("/\s{2,}/", " ", $data);

This replaces two tabs with a single space but leaves one tab as a tab. This seems undesirable.

        $data = preg_replace('{\s+}', ' ', $data);

This code will cause it to sometimes replace a single space with a single space, but it will always replace any amount of whitespace with a single space.

I also changed the double quoted strings to single quoted strings. This saves a step of checking the strings for variables and more clearly expresses what you are doing.

I prefer to use {} as my delimiters in Perl regular expressions. It's often more readable that way. Note that we can get rid of /\ here.

Don't Repeat Yourself (DRY)

You have an awful lot of repeated code that can be replaced by

    $newData = html_entity_decode($data, ENT_HTML5 | ENT_QUOTES, 'UTF-8');

This statement appears in every branch that does not return false, nine times. Once seems to be enough.

    if ( isset($minimum) ) {
        if ( 0 == $minimum ) {
            return false;
        }

        $dataLen = strlen($newData);
        if ( $dataLen < $minimum ) {
            return false;
        }

        if ( isset($maximum) ) {
            if ( $maximum < $minimum || $dataLen > $maximum ) {
                return false;
            }
        }
    }

Note: under some circumstances, it might be worth making this a separate function. It's marginal but worth considering.

Note how it has no interactions with $space or $extends, so we don't have to check those at the same time.

    if ( isset($space) && true === $space ) {
        if ( isset($extends) ) {
            if ( 'EXT_PUNCTUATION' === $extends ) {
                $newData = preg_replace('{[^\p{L}\p{P}\ ]}u', '', $newData);
            } else if ( 'EXT_ANY' === $extends ) {
                $newData = preg_replace('{[^\p{L}\p{P}\p{S}\ ]}u', '', $newData);
            } else {
                return false;
            }
        } else {
            $newData = preg_replace('{[^\p{L}\ ]}u', '', $newData);
        }
    } else {
        $newData = preg_replace('{[^\p{L}]}u', '', $newData);
    }

This replaces nine preg_replace statements with four. That seems a bit better.

If your compiler will support it, I'd prefer the full word Unicode properties to the single letter versions. '{[^\p{Letter}\p{Punctuation}\p{Symbol}\ ]}u'.

    if ( isset($minimum) ) {
        $dataLen = strlen($newData);
        if ( $dataLen < $minimum ) {
            return false;
        }

        if ( isset($maximum) ) {
            if ( $dataLen > $maximum ) {
                return false;
            }
        }
    }

    $newData = htmlentities($newData, ENT_HTML5 | ENT_QUOTES, 'UTF-8');

    return $newData;

The last two statements appear in all nine branches that do not return false, but we now we do them only once.

If I counted properly, this replaces twenty-two return statements with eight.

    }

}

And that's the entire class in 64 lines (as compared to 160 for your version).

Usage

$newString_1 = $_validator->alpha($string);
$newString_2 = $_validator->alpha($string, true);
$newString_3 = $_validator->alpha($string, true, 500);
$newString_4 = $_validator->alpha($string, true, 20, 150);
$newString_5 = $_validator->alpha($string, true, 20, 150, "EXT_PUNCTUATION");
$newString_6 = $_validator->alpha($string, true, 20, 150, "EXT_ANY")

I'm not crazy about this. It's not clear to me what true does or 20 or 150. If you had a validation options class, you could say something like

$options->setSpacesAllowed(true);
$options->setMinimumLength(20);
$options->setMaximumLength(150);
$newString = $_validator->alpha($string, $options);

Which is more verbose but self-commenting.

\$\endgroup\$
  • \$\begingroup\$ Man, many thanks for your answer. Please see my edit and tell me what do you think about that new class.? Also I created the method that way because I need to check multiple strings with differents options in a row, so I don't want to be changing the method options everytime I need to check a new string. \$\endgroup\$ – Victor Tello Jan 26 '15 at 15:58
  • \$\begingroup\$ @VictorTello You should put the new version of the class into a new question, linking to this one for context. For more information, see What you may and may not do after receiving answers. \$\endgroup\$ – Brythan Jan 28 '15 at 7:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.