#235: Add matchers for `Comparable` objects #245

andreoss · 2021-05-09T03:04:41Z

Add matchers for comparable objects backed by https://github.com/hamcrest/JavaHamcrest/blob/master/hamcrest/src/main/java/org/hamcrest/comparator/ComparatorMatcherBuilder.java

codecov · 2021-05-09T03:05:31Z

Codecov Report

Merging #245 (b282a83) into master (37cac65) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #245      +/-   ##
============================================
+ Coverage     98.95%   99.01%   +0.06%     
- Complexity      150      158       +8     
============================================
  Files            28       35       +7     
  Lines           383      407      +24     
  Branches          7        7              
============================================
+ Hits            379      403      +24     
  Misses            4        4

Impacted Files	Coverage Δ	Complexity Δ
...rg/llorllale/cactoos/matchers/MatcherEnvelope.java	`100.00% <ø> (ø)`	`4.00 <0.00> (ø)`
...lorllale/cactoos/matchers/IsComparableEqualTo.java	`100.00% <100.00%> (ø)`	`2.00 <2.00> (?)`
...lale/cactoos/matchers/IsComparableGreaterThan.java	`100.00% <100.00%> (ø)`	`1.00 <1.00> (?)`
...oos/matchers/IsComparableGreaterThanOrEqualTo.java	`100.00% <100.00%> (ø)`	`1.00 <1.00> (?)`
...orllale/cactoos/matchers/IsComparableLessThan.java	`100.00% <100.00%> (ø)`	`1.00 <1.00> (?)`
...actoos/matchers/IsComparableLessThanOrEqualTo.java	`100.00% <100.00%> (ø)`	`1.00 <1.00> (?)`
.../java/org/llorllale/cactoos/matchers/IsNumber.java	`100.00% <100.00%> (ø)`	`1.00 <0.00> (-5.00)`
...rg/llorllale/cactoos/matchers/NaturalOrdering.java	`100.00% <100.00%> (ø)`	`3.00 <3.00> (?)`
...g/llorllale/cactoos/matchers/NumberComparator.java	`100.00% <100.00%> (ø)`	`4.00 <4.00> (?)`
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37cac65...b282a83. Read the comment docs.

0crat · 2021-05-09T04:03:45Z

@victornoel /z everybody who has role REV is banned at #245; I won't be able to assign anyone automatically; consider assigning someone manually (as in §19), or invite more people (as in §51), or remove the job from the scope (as in §14)

victornoel

@andreoss here are a few comments

victornoel · 2021-05-11T15:46:32Z

src/main/java/org/llorllale/cactoos/matchers/IsEqualTo.java

+ * @param <T> Underlying type.
+ * @since 1.0.0
+ */
+public final class IsEqualTo<T extends Comparable<? super T>> extends


@andreoss I'm not totally convinced by that name… especially because there is already a class named like this in hamcrest :/ maybe IsComparableTo? It's not very good either to be honest

@victornoel IsComparableEqualTo, IsComparableGreaterThan, etc?

@andreoss yep, I think it's not bad what you propose, a bit verbose but no ambiguity :) let's that! thx

@victornoel Done

victornoel · 2021-05-11T15:47:53Z

src/main/java/org/llorllale/cactoos/matchers/IsEqualTo.java

+     * @param comparator The comparator.
+     * @param expected The expected value
+     */
+    public IsEqualTo(final Comparator<T> comparator, final T expected) {


@andreoss let's use Comparator<? super T> here

@victornoel Done

src/main/java/org/llorllale/cactoos/matchers/NaturalOrdering.java

victornoel · 2021-05-11T15:51:21Z

src/main/java/org/llorllale/cactoos/matchers/NumberComparator.java

+    /**
+     * Comparator of numbers.
+     */
+    private static final Comparator<Number> FNC =


@andreoss instead of using a static field, let's have a normal field, a private constructor taking the Comparator<Number> and the public constructor just delegates to it by building the actual Comparator.

@victornoel Done.

victornoel · 2021-05-11T15:52:07Z

src/main/java/org/llorllale/cactoos/matchers/NumberComparator.java

+
+    @Override
+    public String toString() {
+        return "NumberComparator{}";


@andreoss let's remove the {} at the end of the string, or is it needed for some reason?

@victornoel This toString is needed for the description, it cannot be disabled by hamcrest.
hamcrest/JavaHamcrest#344

@victornoel Done.

victornoel · 2021-05-11T15:56:16Z

src/test/java/org/llorllale/cactoos/matchers/NaturalOrderingTest.java

+ */
+final class NaturalOrderingTest {
+
+    @MethodSource("arguments")


@andreoss for this kind of source of parameters, I would recommend using CsvSource if possible, just to have everything in one place and not rely on a string to know the name of a method

@victornoel Done

victornoel · 2021-05-13T16:33:54Z

src/main/java/org/llorllale/cactoos/matchers/IsGreaterThanOrEqualTo.java

+import org.hamcrest.comparator.ComparatorMatcherBuilder;
+
+/**
+ * Is {@link Number} greater than or equal to.


@andreoss this is not only about Number, right?

victornoel · 2021-05-13T16:38:34Z

src/main/java/org/llorllale/cactoos/matchers/NumberComparator.java

+    /**
+     * Comparator of numbers.
+     */
+    private final transient Comparator<? super Number> comparator;


@andreoss instead of making this transient, I think it's best to completely make this class non-serializable in practice (not sure why we need them to implement Serializable actually), like we did here: https://github.com/yegor256/cactoos/pull/1587/files#diff-c061f133f1fcdb014d2fd2313288981a4af9ac71ca941c27e6f61b8e6c6fbdbeR38 because either way, objects of this class will never be serializable, but the way I propose seems simpler to understand since all the information is at the class annotations and not distributed in the class fields.

victornoel · 2021-05-13T16:42:40Z

src/main/java/org/llorllale/cactoos/matchers/IsEqualTo.java

+ * @param <T> Underlying type.
+ * @since 1.0.0
+ */
+public final class IsEqualTo<T extends Comparable<? super T>> extends


@andreoss yep, I think it's not bad what you propose, a bit verbose but no ambiguity :) let's that! thx

victornoel · 2021-05-15T13:41:19Z

@rultor merge

rultor · 2021-05-15T13:51:00Z

@rultor merge

@victornoel OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2021-05-15T13:58:51Z

@rultor merge

@victornoel Done! FYI, the full log is here (took me 8min)

0crat · 2021-06-10T08:20:55Z

@victornoel /z everybody who has role REV is banned at #245; I won't be able to assign anyone automatically; consider assigning someone manually (as in §19), or invite more people (as in §51), or remove the job from the scope (as in §14)

andreoss added 3 commits May 8, 2021 23:01

(llorllale#235) Add NaturalOrdering & NumberComparator

8c85d1b

(llorllale#235) Use NaturalOrdering in IsNumber

d0efdb9

(llorllale#235) Add IsEqualTo, IsGreaterThan, IsLessThan & etc.

cf6c44e

(llorllale#235) Fix indentation

c32a8fe

0crat added 0crat/scope 0crat/role/REV labels May 9, 2021

andreoss marked this pull request as ready for review May 9, 2021 03:14

victornoel requested changes May 11, 2021

View reviewed changes

andreoss added 8 commits May 11, 2021 12:41

Generify MatcherEnvelope

35a85c9

Use wildcard in IsEqualTo

dd76900

Pass comparator by ctor, instead of constant

a88c3c8

Fix test

b174616

Remove redundat public modifier

d7afd04

Avoid MethodSource in tests

b2744ce

Remove "{}" from toString

363b291

Suppres FindBugs warning

d63e09d

victornoel requested changes May 13, 2021

View reviewed changes

andreoss added 2 commits May 13, 2021 20:00

(llorllale#235) Add "Comparable" to class names to avoid collisions

22515d3

(llorllale#235) Fix typo

b282a83

rultor merged commit 97d6832 into llorllale:master May 15, 2021

0crat removed the 0crat/scope label Jun 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#235: Add matchers for `Comparable` objects #245

#235: Add matchers for `Comparable` objects #245

andreoss commented May 9, 2021

codecov bot commented May 9, 2021 •

edited

Loading

0crat commented May 9, 2021

victornoel left a comment

victornoel May 11, 2021

andreoss May 11, 2021

victornoel May 13, 2021

andreoss May 14, 2021

victornoel May 11, 2021

andreoss May 11, 2021

victornoel May 11, 2021

andreoss May 11, 2021

victornoel May 11, 2021

andreoss May 11, 2021

andreoss May 11, 2021

victornoel May 11, 2021

andreoss May 11, 2021

victornoel May 13, 2021

victornoel May 13, 2021

victornoel May 13, 2021

victornoel commented May 15, 2021

rultor commented May 15, 2021

rultor commented May 15, 2021

0crat commented Jun 10, 2021

#235: Add matchers for Comparable objects #245

#235: Add matchers for Comparable objects #245

Conversation

andreoss commented May 9, 2021

codecov bot commented May 9, 2021 • edited Loading

Codecov Report

0crat commented May 9, 2021

victornoel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victornoel commented May 15, 2021

rultor commented May 15, 2021

rultor commented May 15, 2021

0crat commented Jun 10, 2021

#235: Add matchers for `Comparable` objects #245

#235: Add matchers for `Comparable` objects #245

codecov bot commented May 9, 2021 •

edited

Loading