Preconditions (for records, or otherwise)
Brian Goetz
brian.goetz at oracle.com
Fri Mar 9 23:24:37 UTC 2018
> Hello,
>
> I'd claim it's an uncontroversial best practice that method and
> constructor parameters should be aggressively *checked for validity*,
> especially when that data is stored and used later (when knowledge of
> where that bad value came from has vanished).
Yes, and especially so for immutable objects (which we'd like to nudge
towards somehow, such as making `final` the default for record fields
and requiring an explicit `non-final`.)
I think what you're getting at is that failing to validate in records
will become an attractive nuisance; because we've stripped away all the
boilerplate, reconstructing a place to hang validation code will be on
the wrong side of the activation energy leap, and anything we can do to
reduce the cost of responsible validation is worth doing. I tend to
agree; if people perceive validation as inconvenient, they won't do it,
and our data will be of lower quality. Which is not the goal ;)
> One thing I've been pushing for as a result is that the design of
> records really, really should not impose a disproportionate penalty to
> adding the first bit of validation. If to check that a number is
> positive I have to change
>
> record Foo(int num, String unrelated) {}
>
> to
>
> record Foo(int num, String unrelated) {
> Foo(int num, String unrelated) {
> if (num <= 0) ...;
> default.this(num, unrelated);
> }
> }
>
> ... then I'd say that the cost of listing my fields three times
> instead of once is too great, and the user may not bother. For
> records, the right amount of repetition really is no repetition.
In general, I am wary about arguments that sound like catering to Billy
Boilerplate, but in this case I agree -- validation is so important, we
don't want to give people any excuses to not do it.
First, let's catalog the repetition.
I think we can surely eliminate the `default.this(num, unrelated)` call;
the parser production can tell the difference between a constructor body
that contains an explicit constructor call (super/this/default.this) and
one that does not. So if the user provides a constructor without an
explicit constructor call, we can just fill in the default
initialization. (There are arguments in favor of both putting it at the
beginning and at the end; I'll take that up separately.) That gets us
down to:
record Foo(int num, String unrelated) {
Foo(int num, String unrelated) {
if (num <= 0) ...;
}
}
Which is slightly better. The remaining repetition is the argument list
of the constructor is repeated. On the one hand, this seems at first to
be incompressible: Java members have signatures. But if we're willing
to do something special for records, we can take advantage of the fact
that a record _must_ have a constructor (the "primary constructor") that
matches the record signature, and let the user leave this out if they
are declaring an explicit primary constructor:
record Foo(int num, String unrelated) {
Foo{
if (num <= 0) ...;
}
}
or
record Foo(int num, String unrelated) {
primary-constructor {
if (num <= 0) ...;
}
}
or similar could be a shorthand for declaring the full primary
constructor signature. I think that gets you down to your minimum,
without losing much in the way of readability.
> We've discussed addressing this in either a *records-specific* or a
> generalized way. The former is (imho) the least we can do to satisfy
> "first do no harm". This could be a matter of saying that a record's
> primary constructor gets to have various uninteresting boilerplate be
> *inferred* (though if we had a way to also get around the traditional
> annoyance of parameters and fields both being in scope at the same
> time with the same names, that might be even better). So I'd like to
> figure out what that would look like. (As a side product, maybe this
> solution solves the question of "where does a constructor annotation
> go?".)
The above is a record-specific approach, which takes advantage of the
fact that a record must have a constructor that matches the record
signature.
I am a little more wary of dealing with the "pun" between the fields and
the constructor arguments in a record-specific way, since I'd like for
there to be a simple mechanical refactoring between records and classes
that could be records.
If the compiler is going to fill in the super call / field
initialization, we have two choices: fill them in at the beginning, or
at the end, of the constructor. Both have pros and cons.
General: we can lift the restriction of "no statements before
this/super" (for years, we couldn't, until some verifier improvements
made this possible). In that case, statements before the constructor
call have `this` as DU, so you can't do field access in that part of the
ctor, you can only work on the arguments:
Foo(int x, int y) {
if (x < y) throw ... // `this` is DU
this(...); // now `this` is DA
more stuff
}
If we are presented with
Foo(int num, String unrelated) {
if (num <= 0) ...;
}
we could desugar that into
Foo(int num, String unrelated) {
if (num <= 0) ...;
default.this(num, unrelated);
}
or
Foo(int num, String unrelated) {
default.this(num, unrelated);
if (num <= 0) ...;
}
Checking preconditions after you create the object feels wrong to me, so
the former speaks to me. It also has the benefit that you can normalize
the fields without having to explicitly provide the default call:
Foo(int num, String unrelated) {
if (num <= 0) num = 0;
// implicit default, writes normalized num to this.num
}
The only time you'd need an explicit default under this interpretation
is if you had additional fields to initialize in the constructor.
Putting the implicit default first is more like how things work now, but
it means you can only check the values after you write the fields, and
you can't normalize them without an explicit default.
> Lastly... hey, what about just a *library* like Guava's Preconditions
> class? I made that thing, and it is extremely popular here. It also
> gives extraordinarily small benefit. Yeah, it lets you express your
> expectation positively instead of negatively. It lets you create a
> message with %s. That's about it. Yawn.
I'd be in favor; seems a pretty big return-on-complexity. The closest
thing we have in JDK now is Objects.requireNonNull(), which I use a lot
but is obviously a one-trick pony.
Other ideas along these lines include:
- A statement, analogous to assert, but which is unconditional
("validate");
- Compiler and documentation heuristics for Preconditions, where, if a
method begins with a block of preconditions, they are propagated into
the documentation;
- Better Javadoc support for describing preconditions, so they don't
have to be spelled out longhand.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/amber-spec-experts/attachments/20180309/2b105d1d/attachment-0001.html>
More information about the amber-spec-experts
mailing list