beberlei / assert

Thin assertion library for use in libraries and business-model
Other
2.41k stars 186 forks source link

Improve propertiesExist assertion #219

Closed bicpi closed 7 years ago

bicpi commented 7 years ago

From reading the code of the propertiesExists() assertion I thought it might not work as intended.

As the foreach loop breaks at the first invalid property encountered it hardly makes sense to store the invalid properties in an array. First collecting all the properties and then displaying all of them is also what I'd expect as a user.

Moreover and for consistency reasons, I added the isObjectOrClass() check at the beginning –which is also present in the propertyExists() method.

(usually I like more fine grained unit testing including the exception messages etc. but I didn't want to break the current pattern; if desired I can add some more of course)

rquadling commented 7 years ago

@bicpi Thank you for your PR. If you want to improve the unit tests, please do. Always happy to accept contributions with unit tests!

bicpi commented 7 years ago

@rquadling Didn't wanted to be too intrusive, just made sure that

abacaphiliac commented 7 years ago

👍