Improve naming of assorted Refaster templates and associated tests#28
Improve naming of assorted Refaster templates and associated tests#28rickie wants to merge 2 commits into
Conversation
| @@ -657,8 +656,7 @@ void after(Map<K, V> map) { | |||
| } | |||
| } | |||
|
|
|||
| // XXX: Find a better name. | |||
| static final class AssertThatMapIsNotEmpty2<K, V> { | |||
| static final class AssertThatIsNotEmpty<K, V> { | |||
| @@ -615,8 +615,7 @@ void after(AbstractMapAssert<?, ?, K, V> mapAssert) { | |||
| } | |||
| } | |||
|
|
|||
| // XXX: Find a better name. | |||
| static final class AssertThatMapIsEmpty2<K, V> { | |||
| static final class AssertThatIsEmpty<K, V> { | |||
There was a problem hiding this comment.
Most likely we want different names for this in the future, but for now I followed the naming scheme.
There was a problem hiding this comment.
Hmm, AssertThatMapIsEmpty would be better (and possible now that the other method is renamed).
07a25cf to
e747a47
Compare
Stephan202
left a comment
There was a problem hiding this comment.
Rebased and added a commit. Seems to require a bit more thinking 🤔
| @@ -615,8 +615,7 @@ void after(AbstractMapAssert<?, ?, K, V> mapAssert) { | |||
| } | |||
| } | |||
|
|
|||
| // XXX: Find a better name. | |||
| static final class AssertThatMapIsEmpty2<K, V> { | |||
| static final class AssertThatIsEmpty<K, V> { | |||
There was a problem hiding this comment.
Hmm, AssertThatMapIsEmpty would be better (and possible now that the other method is renamed).
| @@ -657,8 +656,7 @@ void after(Map<K, V> map) { | |||
| } | |||
| } | |||
|
|
|||
| // XXX: Find a better name. | |||
| static final class AssertThatMapIsNotEmpty2<K, V> { | |||
| static final class AssertThatIsNotEmpty<K, V> { | |||
| @@ -186,7 +186,7 @@ OptionalDouble after(DoubleStream stream) { | |||
| } | |||
|
|
|||
| /** Prefer {@link DoubleStream#noneMatch(DoublePredicate)} over more contrived alternatives. */ | |||
| static final class DoubleStreamNoneMatch { | |||
| static final class DoubleStreamNoneMatchDoublePredicate { | |||
There was a problem hiding this comment.
Here and below: I wonder whether in this case the original naming is more natural, since the lambda expression below also represents a DoublePredicate. (Or maybe we can do something more creative 🤔.)
e747a47 to
395d266
Compare
|
I'd suggest to close this PR. Once we work on the BugPattern that'll enforce a specific naming scheme for Refaster templates, we can use the WDYT @Stephan202 ? |
|
I'm fine closing this PR as-is, though OTOH such a checker could save us quite some time when open-sourcing the code base. How was the current changeset generated, and could we use that as the foundation of a new checker? |
|
The current changeset was done manually (according to the ideas we had at the time about the naming pattern). |
This PR contains code of #25 . We want to prepare the Refaster templates such that we can easily add the annotations
@Template{,Collection}in the appropriate places.In the afternoon we talked about the reordering of some Refaster templates. I considered doing that in a second commit, but checked the files again, and saw that there is no alphabetical ordering currently, so I left it out.
Also, in the future we most likely want to automate the ordering with another BugPattern 😉, so did not bother to do it right now.
Note: for some Refaster templates our "current" naming scheme does not suffice.