Skip to content
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

equalTo() Should Not Force Type Match of Actual to Compile #59

Closed
EarthCitizen opened this issue Sep 8, 2014 · 6 comments
Closed

equalTo() Should Not Force Type Match of Actual to Compile #59

EarthCitizen opened this issue Sep 8, 2014 · 6 comments

Comments

@EarthCitizen
Copy link

Currently, equalTo() will not compile when comparing Object to another class forcing unnecessary casting. For example:

HttpServletRequest request = (get request);
Object actual = request.getAttribute("SomeAttribute");
assertThat(actual, equalTo((Object) fooInstance));

In this example, the compile will fail unless fooInstance is cast to Object. But this is unnecessary for equals() to work correctly.

If the signature of equalsTo() were:

Matcher<Object> equalTo(Object o)

it could avoid the above problem.

@sf105
Copy link
Member

sf105 commented Sep 8, 2014

you have a point. There are tradeoffs in that the typed version will catch some mistakes at compile time and works better with jmock (our original client). I'm now beginning to think that jMock should have had its own matcher methods, but it's a bit late now.

@EarthCitizen
Copy link
Author

What will break if the signature of equalTo() is changed now? Would it affect a small number of users or create a large inconvenience for most?

@sf105
Copy link
Member

sf105 commented Sep 8, 2014

People using asserThat directly will be fine, apart from occasionally missing a helpful type clash when refactoring. People using Hamcrest with JMock (and maybe other mock frameworks) will get compilation failures.

@EarthCitizen
Copy link
Author

What do you think a good solution would be?

@npryce
Copy link
Contributor

npryce commented Nov 23, 2014

The only solution is to add another matcher factory method. E.g.

isEqualToObject(Object o) -> Matcher

npryce added a commit that referenced this issue Nov 23, 2014
@npryce
Copy link
Contributor

npryce commented Nov 23, 2014

Implemented in master.

@npryce npryce closed this as completed Nov 23, 2014
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

No branches or pull requests

3 participants