Skip to content

Rewrite HasPropertyWithValue to be a FeatureMatcher#96

Closed
tomwhoiscontrary wants to merge 0 commit intohamcrest:masterfrom
tomwhoiscontrary:master
Closed

Rewrite HasPropertyWithValue to be a FeatureMatcher#96
tomwhoiscontrary wants to merge 0 commit intohamcrest:masterfrom
tomwhoiscontrary:master

Conversation

@tomwhoiscontrary
Copy link

HasPropertyWithValue and FeatureMatcher look really similar to me, and it was driving me nuts that they're completely unrelated. So i made them related.

I have no idea if this is something you would actually want to merge, but i thought someone might at least find the idea mildly interesting.

Because FeatureMatcher has its own ideas about how mismatch descriptions look, i have changed the messages expected in the tests for HasPropertyWithValue. I think the new messages look at least as good as the old ones.

One source of complication in this was that FeatureMatcher::featureValueOf has no way to fail cleanly, so i introduced an Either-like type to wrap the extracted feature value (or its absence). This whole thing would be briefer and more idiomatic-for-Java if FeatureMatcher::featureValueOf was able to throw some sort of exception to signal that a feature's value could not be obtained. FeatureMatcher::matchesSafely could catch this and use its message as a mismatch description. This change would not be strictly backwards-compatible, but the only code which would suffer would be that calling FeatureMatcher::featureValueOf, which would be an odd thing to do.

@tomwhoiscontrary
Copy link
Author

I've taken this commit off my master so i can do other stuff. It's still around, on a different branch:

https://github.com/tomwhoiscontrary/JavaHamcrest/tree/pr-Rewrite-HasPropertyWithValue-to-be-a-FeatureMatcher

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.

1 participant

Comments