Sounds reasonable and shares some goals with JEP358 [1] so why not. [1] https://openjdk.java.net/jeps/358
On Wed, Dec 9, 2020 at 8:26 AM Vladimir Sitnikov < [email protected]> wrote: > Hi, > > I suggest we use requireNonNull(var, "var") pattern rather > than requireNonNull(var). > > Then error message would include which variable/field turned out to be > null. > It would make a big difference in case a constructor verifies multiple > parameters like in Correlate: > > protected Correlate( > RelOptCluster cluster, > RelTraitSet traitSet, > RelNode left, > RelNode right, > CorrelationId correlationId, > ImmutableBitSet requiredColumns, > JoinRelType joinType) { > super(cluster, traitSet, left, right); > assert !joinType.generatesNullsOnLeft() : "Correlate has invalid join > type " + joinType; > this.joinType = requireNonNull(joinType); > this.correlationId = requireNonNull(correlationId); > this.requiredColumns = requireNonNull(requiredColumns); > > If a user makes a mistake and passes null to joinType or correlationId, > then Calcite would throw NPE, however, it would be hard to tell which > parameter was null. > Line numbers are known to drift over time. > > I suggest we use the following pattern consistently: > > this.joinType = requireNonNull(joinType, "joinType"); > this.correlationId = requireNonNull(correlationId, "correlationId"); > this.requiredColumns = requireNonNull(requiredColumns, > "requiredColumns"); > > The PR is https://github.com/apache/calcite/pull/2293 > > The suggested change automatically aligns single-word message with the > first argument to requireNonNull. > In other words, "./gradlew style" command would insert/fix the messages > automatically. > I assume modern IDEs enable developers to configure var => > requireNonNull(var, "var") expansion (I use it in IDEA). > > Currently we have > 545 usages of requireNonNull(var) <-- this is to be updated to (var, "var") > 359 usages of requireNonNull(var, "var") > 37 usages of requireNonNull(var, () -> ..) > 33 usages of requireNonNull(var, "custom message") > > If no-one objects within three days, I'll assume lazy consensus and commit > it. > > Vladimir >
