Re: Handling nulls for cumulative task arg values.
Locked
Re: Handling nulls for cumulative task arg values.
 
|
On Mon, Mar 3, 2014 at 9:21 AM, Luke Daley <[hidden email]> wrote: -- Hi, -1 2. Should we just silently drop nulls? -1 3. Should we establish patterns/methods for validation and better error messages in this case? +1 IMHO, Ignoring nulls unavoidably leads inconsistent apis because we won't be able to ignore them everywhere.
Cheers! Szczepan Faber Principal engineer@gradle; Founder@mockito
|
|
3. Should we establish patterns/methods for validation and better error messages in this case? +1 from me too. One option would be to use bean validation 1.1 on public API methods. If gradle can inject the validator appropriately, then this would be very powerful, as gradle could also catch the validation exceptions for better presentation, translations can be provided, etc.
throw new NullPointerException("JavaExec cannot be given null args") or throw new IllegalArgumentException("JavaExec cannot be given null args") I tend to prefer the IllegalArgumentException. In my experience developers assume that a NullPointerException is only thrown by the JVM and is a sign that the called code is "bad", whereas an IllegalArgumentException makes it clear that the API didn't like what the client gave it. Also, you can be consistent and use IllegalArgumentException for other validation checks (empty strings, negative values, etc.).
But bean validation 1.1 would be awesome. Regards Rob |
Re: Handling nulls for cumulative task arg values.
|
|
In reply to this post by Szczepan Faber-2
On 3 Mar 2014, at 7:21 pm, Luke Daley <[hidden email]> wrote: Hi, No 2. Should we just silently drop nulls? No 3. Should we establish patterns/methods for validation and better error messages in this case? Yes. I’d change the decoration and/or DSL layer to deal with this as a cross-cutting thing. We also want to something very similar for dealing with mutability during the object’s lifecycle. -- Adam Murdoch Gradle Co-founder http://www.gradle.org VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting http://www.gradleware.com |
Re: Handling nulls for cumulative task arg values.
|
|
> Adam Murdoch <mailto:[hidden email]> > 4 March 2014 8:19 am > > On 3 Mar 2014, at 7:21 pm, Luke Daley <[hidden email] > <mailto:[hidden email]>> wrote: > >> Hi, >> >> |task blowUp(type: JavaExec) { >> main = 'MainClass' >> classpath = sourceSets.main.runtimeClasspath >> args null >> }| >> >> This produces a less-than-user-friendly NPE: >> >> |Caused by: java.lang.NullPointerException >> at org.gradle.util.GUtil.addToCollection(GUtil.java:135) >> at >> org.gradle.process.internal.JavaExecHandleBuilder.args(JavaExecHandleBuilder.java:179) >> at org.gradle.api.tasks.JavaExec.args(JavaExec.java:261) >> at >> build_7p31187sv6j4incv2nd9fufiog$_run_closure1.doCall(/home/kamils/dev/sandbox/gradle/null_arg/build.gradle:6)| >> >> >> We have a PR that proposes just silently ignoring nulls here and in >> other similar cases: https://github.com/gradle/gradle/pull/251 >> >> I'm not sure what we want to do here. Silently ignoring nulls doesn't >> sit well with me. I'd prefer we validate the args early and not allow >> nulls. We don't seem to do this anywhere though that I could find. >> >> So: >> >> 1. Is this current situation good enough? > > No > >> 2. Should we just silently drop nulls? > > No > >> 3. Should we establish patterns/methods for validation and better >> error messages in this case? > > Yes. > > I’d change the decoration and/or DSL layer to deal with this as a > cross-cutting thing. We also want to something very similar for > dealing with mutability during the object’s lifecycle. close saying we want to fix this more thoroughly in the future. There's a low chance the OP will want to help on this, but I'll propose. > > -- > Adam Murdoch > Gradle Co-founder > http://www.gradle.org > VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting > http://www.gradleware.com > > > > Luke Daley <mailto:[hidden email]> > 3 March 2014 6:21 pm > Hi, > > |task blowUp(type: JavaExec) { > main = 'MainClass' > classpath = sourceSets.main.runtimeClasspath > args null > }| > > This produces a less-than-user-friendly NPE: > > |Caused by: java.lang.NullPointerException > at org.gradle.util.GUtil.addToCollection(GUtil.java:135) > at > org.gradle.process.internal.JavaExecHandleBuilder.args(JavaExecHandleBuilder.java:179) > > at org.gradle.api.tasks.JavaExec.args(JavaExec.java:261) > at > build_7p31187sv6j4incv2nd9fufiog$_run_closure1.doCall(/home/kamils/dev/sandbox/gradle/null_arg/build.gradle:6)| > > > > We have a PR that proposes just silently ignoring nulls here and in > other similar cases: https://github.com/gradle/gradle/pull/251 > > I'm not sure what we want to do here. Silently ignoring nulls doesn't > sit well with me. I'd prefer we validate the args early and not allow > nulls. We don't seem to do this anywhere though that I could find. > > So: > > 1. Is this current situation good enough? > 2. Should we just silently drop nulls? > 3. Should we establish patterns/methods for validation and better > error messages in this case? --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
«
Return to gradle-dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |