Fluent API for Preconditions

[gissuebot]

Original issue created by matthias.kegele on 2014-05-23 at 07:36 AM

---

Similar to the assertJ library (for checking post conditions of test runs), I would suggest some additional methods for the Preconditions class to make checking arguments easier.

http://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html

So instead of StringAssert assertThat(String value) throws AssertionException StringCheck checkThat(String value) throws IllegalArgumentException, NullPointerException is defined.

AssertJ defines assertions for basic data types and collections. Each assert type defines different method chains are possible.

Example: StringCheck checkThat(String value) checkThat(aString).isNotEmpty().containsIgnoringCase("")...;

[gissuebot]

Original comment posted by Wolfram.Nickl on 2014-05-23 at 07:40 AM

---

Vote! this would really be a nice feature to get the code more readable.

Eg. public void bla(List<Blub> list) {  checkThat(list).describedAs("list has to be not null and not empty").isNotNull().isNotEmpty(); }

instead of: public void bla(List<Blub> list) {  checkNotNull(list, "list null");  checkArgument(!list.isEmpty(), "list empty"); }

This feature would really be appreciated!

[gissuebot]

Original comment posted by lowasser@google.com on 2014-05-23 at 07:45 AM

---

The Truth library (https://github.com/truth0/truth), which shares some of Guava's developers, might be able to support this as a test verb option, almost without modification.

Off the top of my head, though, I am concerned that the performance cost of a fluent API like this would be unacceptable for most applications of preconditions. checkThat(list).describedAs("list has to be not null and not empty").isNotNull().isNotEmpty() essentially must allocate four objects, and preconditions are generally expected to be cheap. (To be fair, Guava's Preconditions incurs varargs overhead, but this is at most one object.)

[gissuebot]

Original comment posted by Wolfram.Nickl on 2014-05-23 at 09:02 AM

---

Ok, thanks for the comment. I understand that performance is relevant.

Perhaps this can be seen from different point of views: performance vs. code-style...

I personally like e.g. assertJ but performance in unit tests is not as critical as at product code.

thanks anyway.

[gissuebot]

Original comment posted by matthias.kegele on 2014-05-26 at 09:34 AM

---

Thank you all for your feedback! I will definetly take a look at the Truth library.

I understand that performance is critical. Wouldn't it also work with just one simple object which is reused for all checks ("return this")? The only property which has to be hold as a state is the describedAs text as far as I can see that. Ignoring that, a StringCheck does not keep any state...

[gissuebot]

Original comment posted by lowasser@google.com on 2014-05-26 at 09:48 AM

---

At least two objects, then: the reference to the string itself, and the description text. But more complex conditions on Iterables will probably need more objects to be useful.

[hakanai]

I'm not sure about this example:

public void bla(List<Blub> list) {
 checkThat(list).describedAs("list has to be not null and not empty").isNotNull().isNotEmpty();
}

I'd rather see:

public void bla(List<Blub> list) {
 checkThat(list).describedAs("list").isNotNull().isNotEmpty();
}

The library can surely handle a little string concatenation to produce a meaningful error message with this much information.

(This is also my main beef with Preconditions.check methods in general. I can only choose between two equally unacceptable options: (a) checkNotNull(value), which as a library user, doesn't tell me which parameter was null, or (b) checkNotNull(value, "value is null"), which forces me as a library developer to say what I'm checking twice, plus adding a caveat that someone has to update both in sync, or introduce an incorrect error message. FWIW, my other beef is that it throws the wrong exception. If I wanted a NullPointerException, I could just not check the parameter!)

[kluever]

We have a rough proposal for this internally, but one of our primary concerns is allocation/performance (since this will primarily be used in production code (e.g., at the top of a constructor))