Style Guide: Assert Section

[Samuel Huang]

Hi,

I was reading 

https://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md

And noticed that in the "Asserts" section we have:

Example assert:

assert someCallWithSideEffects() : "assert description";

Example use of DCHECK_IS_ON:

if (org.chromium.base.BuildConfig.DCHECK_IS_ON) {
   if (!someCallWithSideEffects()) {
     throw new AssertionError("assert description");
   }
}

Should "someCallWithSideEffects()" be "someCallWithoutSideEffects()" instead? Otherwise when assert lines are stripped, the side effects would disappear, leading to different behavior between Debug and Release builds?

--

Samuel Huang

[Andrew Grieve]

Hmm, yeah, the text above those examples reads quite well to me, but I agree the examples don't really make sense.

The first example, agree it should be "Without".

The second example should maybe just say:

if (org.chromium.base.BuildConfig.DCHECK_IS_ON) {
    // Any code here will be stripped in release.
}

--
You received this message because you are subscribed to the Google Groups "java" group.
To unsubscribe from this group and stop receiving emails from it, send an email to java+uns...@chromium.org.
To post to this group, send email to ja...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CAMve_ytGbQgKO%3DpQTMEa-okPTvQYRexMNqDEU3m_%2BuwhcLo6yA%40mail.gmail.com.

[Tommy Nyquist]

Agreed. I'd be happy to review a patch for this. I think Andrew's suggestion for the DCHECK_IS_ON example is helpful too.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CABiQX1U%2BVXTze_hrhoV3G7VphJBsdLSZfnFAbcOuXhuEpE-Ljw%40mail.gmail.com.