FDMediagroep / hamcrest-jsoup

hamcrest matchers for jsoup
GNU General Public License v2.0
7 stars 2 forks source link

Naming inconsistent with core Hamcrest matchers #2

Open fransflippo opened 9 years ago

fransflippo commented 9 years ago

Hamcrest matchers are generally named so that an assertThat using the matchers reads like normal English, eg.:

assertThat(myString, startsWith("hello w");
assertThat(myList, contains("a", "b"));

The JsoupMatchers currently don't have this:

assertThat(link, withAttribute("href", "http://trifork.nl");
assertThat(p, withText("Hello world"));

I propose to change this. The old versions can be deprecated and removed in the next major version (I guess that would be 1.0). The new names should make sense combined with assertThat() and be succinct but sufficiently descriptive, e.g. isElementWithText or hasText instead of withText.

My proposal:

Let me know what you think in a comment.

Krijger commented 9 years ago

I don't have my code with me but there was a reason for these names I remember. It has to do with the structure of building the matcher that wraps other matchers so that the HTML would only have to be parsed once. A refactor of names would need to check that use cade as well. If it works there as well I have no objections what so ever.

Also note that the reason that this is a version 0.x is not having to deprecate yet. You can just change since I think FD is currently the only user. When arriving at a nice 1.0 I still think we should donate to jsoup or hamcrest.

Krijger commented 9 years ago

I now see you propose "selecting" for the wrapping thingy. If that works then I agree. No emotional attachments to my choices here :)

Only, I vote in favor of keeping hasHref

OrangeDog commented 5 years ago

This is done isn't it?

jmsnoeij commented 5 years ago

I see that most operation have been renamed, but the classes themselves still have the old name. Internally we rarely use this artifact any more (as the number of recent changes might suspect) so I'm not sure if all operations are renamed.