CODEWITHSUNDEEP
phpcakephp
yossi
yossi

Reputation: 3164

CakePHP - a code sample seems strange to me, what am i missing?

Attached code taken from cakephp bakery, where someone uploaded a sample about custom validation rules.

class Contact extends AppModel
{
    var $name = 'Contact';
    var $validate = array(
        'email' => array(
            'identicalFieldValues' => array(
                 'rule'    => array('identicalFieldValues', 'confirm_email' ),
                 'message' => 'Please re-enter your password twice so that the values match'
                )
            )
        );


    function identicalFieldValues( $field=array(), $compare_field=null ) 
    {
        foreach( $field as $key => $value ){
            $v1 = $value;
            $v2 = $this->data[$this->name][ $compare_field ];                 
            if($v1 !== $v2) {
                return FALSE;
            } else {
                continue;
            }
        }
        return TRUE;
    }

}

In the code, the guy used a foreach to access an array member which he had its name already! As far as I understand - it's a waste of resources and a bad(even strange) practice.

One more thing about the code: I don't understand the usage of the continue there. it's a single field array, isn't it? the comparison should happen once and the loop will be over. Please enlighten me.

Upvotes: 0

Views: 551

Answers (1)

NullUserException
NullUserException

Reputation: 85478

In the code, the guy used a foreach to access an array member which he had its name already! As far as I understand - it's a waste of resources and a bad(even strange) practice.

The first parameter is always an array on one key and its value, the second parameter comes from the call to that function, in a block named as the key... So, all you need is to send the key and no need to iterate

The code uses foreach to iterate through $field, which is an array of one key value pair. It all starts when the validation routine invokes identicalFieldValues, passing it two values - $field, which would be an array that looks like:

array ( 
    [email] => 'user entered value 1'
)

The second parameter $compare_field would be set to the string confirm_email.

In this particular case, it doesn't look like it makes a lot of sense to use foreach since your array only has one key-value pair. But you must write code this way because CakePHP will pass an array to the method.

I believe the reason why CakePHP does this is because an array is the only way to pass both the field name and its value. While in this case the field name (email) is irrelevant, you might need in other cases.

What you are seeing here is one of the caveats of using frameworks. Most of the time, they simplify your code. But sometimes you have to write code that you wouldn't write normally just so the framework is happy.

One more thing about the code: I don't understand the usage of the continue there. it's a single field array, isn't it? the comparison should happen once and the loop will be over. Please enlighten me.

Indeed. And since there are no statements in the foreach loop following continue, the whole else block could also be omitted.

A simplified version of this would be:

function identicalFieldValues($field=array(), $compare_field=null) 
{
    foreach ($field as $field) {

        $compare = $this->data[$this->name][$compare_field];                 
        if ($field !== $compare) {
            return FALSE;
        } 
    }
    return TRUE;
}

And I agree with you, the loop only goes through one iteration when validating the email field. regardless of the field. You still need the foreach because you are getting an array though.

Upvotes: 1

Related Questions