Skip to content

Conversation

@slavcodev
Copy link
Contributor

I think the short form of array is not difficult to write than a comma separated string. But we gain a little bit in performance when creating each model instance.

public function rules()
{
    return [
        [['name', 'email', 'subject', 'body'], 'required'],
    ];
}

vs

public function rules()
{
    return [
        ['name, email, subject, body', 'required'],
    ];
}

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 1c2489a on slavcodev:only-arrays-in-rules into 6066c29 on yiisoft:master.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'username' is fine in this case. Should also fix Gii model generator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes single attribute may be as string, but I think it is not necessary to teach programmers to poor :)
I would even removed the conversion to the array for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remove conversion to array and add type hinting into Validator::createValidator, please tell me your opinion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo it should be possible to specify one attribute without brackets. I see no sense in having to add [] each time when one attribute rule is specified. Nearly 3/4 of the rules I created today(created a bunch of new models ;-) ) where only for one attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I agree. But I propose move conversion in Model::createValidators

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ecc557e on slavcodev:only-arrays-in-rules into 6066c29 on yiisoft:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling cbca145 on slavcodev:only-arrays-in-rules into 6066c29 on yiisoft:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 9bf7f79 on slavcodev:only-arrays-in-rules into 6066c29 on yiisoft:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3ff8f10 on slavcodev:only-arrays-in-rules into 6066c29 on yiisoft:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.43%) when pulling 2adcd16 on slavcodev:only-arrays-in-rules into 6066c29 on yiisoft:master.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed earlier, for single attribute, we prefer to use a string rather than an array. Could you please fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@samdark
Copy link
Member

samdark commented Nov 13, 2013

I'm for taking this change. With 5.4 array syntax it looks much better than strings.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) when pulling f57b353 on slavcodev:only-arrays-in-rules into 6066c29 on yiisoft:master.

qiangxue added a commit that referenced this pull request Nov 13, 2013
Using only arrays in rules instead comma-separated string
@qiangxue qiangxue merged commit 447bad5 into yiisoft:master Nov 13, 2013
@qiangxue
Copy link
Member

Thank you very much!

@slavcodev slavcodev deleted the only-arrays-in-rules branch November 13, 2013 15:56
cebe added a commit that referenced this pull request Nov 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants