Fix bug with automatic adding of table name

[jordanlev]

The result of explode(',', substr($rule, 7)) on line 420 is always an array, so isset($parameters[0]) always returns true. This change makes it so the check on line 423 looks for an empty string as well, which is what happens when the standalone 'unique' rule is declared.

[dwightwatson]

Hey, thanks! If explode() always returns an array, then there is no point checking isset($parameters[0]) at all, right? Should it only check strlen($parameters[0]) === 0 instead?

[jordanlev]

In theory, I suppose. When I wrote "always" I meant "always in the testing I did yesterday" :)

I left the isset check just in case there's some edge case I'm unaware of. I'm new to Laravel, so I didn't want to make too many assumptions about what would be passed in.

But yeah, implode always returns an array of strings, right? (Never an array of arrays, or an empty array). So yeah, should be safe.

(I avoided doing "empty()" in case someone bizarrely named their table "0" or "false").

On Nov 17, 2015, at 6:03 PM, Dwight Watson notifications@github.com wrote:

Hey, thanks! If explode() always returns an array, then there is no point checking isset($parameters[0]) at all, right? Should it only check strlen($parameters[0]) === 0 instead?

— Reply to this email directly or view it on GitHub.

[jordanlev]

Oops, in my prior comment I meant to say "explode", not "implode". From the php manual:

If delimiter is an empty string (""), explode() will return FALSE. If delimiter contains a value that is not contained in string and a negative limit is used, then an empty array will be returned, otherwise an array containing string will be returned.

Since you are using a delimiter and no limit, the yes we can assume a 1-element array containing an empty string will always be returned if an empty string is the input (which it would be after the substr call above on just the word "unique").

On Nov 17, 2015, at 6:03 PM, Dwight Watson notifications@github.com wrote:

Hey, thanks! If explode() always returns an array, then there is no point checking isset($parameters[0]) at all, right? Should it only check strlen($parameters[0]) === 0 instead?

— Reply to this email directly or view it on GitHub.

[dalabarge]

It can simply read:

if ( strlen($parameters[0]) === 0 ) {
    $parameters[0] = $this->getModel()->getTable();
}

As you state it always returns an array. The reason for the parameter check was a matter of consistency I think with the next condition where it's checking for the presence of index of one (which does not always exist). I'd be happier with if( empty($parameters[0]) ) { myself as hand-holding for null, false, 0, and other such "empty" values should not be a thing. No sane developer should be doing this and it's a cheaper calculation internally.

[dwightwatson]

Yeah, let's drop the isset() then and just use empty() if it's faster. If you can update the PR I'll merge it in and tag it!

[jordanlev]

See #142 (I couldn't figure out how to modify a pull request so I just made a new one). Thanks!

[dalabarge]

@jordanlev just make more commits and push to the same branch your PR was for: jordanlev:patch-1. They'll come across as new commits to the same PR. If you undo a previous commits it's often a good idea to also squash the commits so you have a single change represented. #143 looks good to go though.