-
Notifications
You must be signed in to change notification settings - Fork 152
fix isA*Of exception messages #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Could you please rebase this PR? |
7450583 to
37db12a
Compare
|
PR is rebased |
9ca859e to
e7c5462
Compare
|
I also changed some error messages. You may have a look again? |
| $message ?: 'Expected an instance of this class or to this class among his parents %2$s. Got: %s', | ||
| static::typeToString($value), | ||
| $message ?: 'Expected an instance of this class or to this class among its parents "%2$s". Got: %s', | ||
| static::valueToString($value), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still feel that typeToString() is correct here.
I want to see string, AcmeService or boolean. I don't want to see the string value. Do I?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think it depends whether the value is passed as an object or string. WDYT about the following?
$isValueAString = \is_string($value);
if (!\is_a($value, $class, $isValueAString)) {
static::reportInvalidArgument(sprintf(
$message ?: 'Expected an instance of this class or to this class among its parents "%2$s". Got: %s',
$isValueAString ? static::valueToString($value) : static::typeToString($value),
$class
));
}An assertion about checking the type of value whether it is an object or string might be also a good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the invalid test cases with passing an object and static::valueToString($value) is correct here.
355d491 to
00c780f
Compare
00c780f to
aede6e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this.
No description provided.