7 ms·
> I'm surprised it stayed uncaught for so long. I'm not surprised. Java 9 has minor adoption, and having side effects on the left hand side of an assignment op
by dtech 8y ago
> I'm surprised it stayed uncaught for so long.
I'm not surprised. Java 9 has minor adoption, and having side effects on the left hand side of an assignment operation is really really bad practice.
So you'd need a codebase that (1) was updated or written in a very new Java version, (2) contained the smelly code and (3) had a harmful double side-effect that got noticed. That's a pretty rare combination.
- coredog64 8y agoOracle broke compression in Tomcat 7.x in 8u151/152 and didn't fix it until 171/172 [0]. That was 6 months, so nothing Oracle does surprises me. [0] Allegedly the fix was available in a non-public build of 8u162
- dionian 8y agoI dunno if this is entirely Oracle's fault though... https://bugs.java.com/view_bug.do?bug_id=JDK-8189789 https://bugs.java.com/view_bug.do?bug_id=JDK-8189789
- kllrnohj 8y ago> having side effects on the left hand side of an assignment operation is really really bad practice. No it isn't. Something like this: data[index++] += 'a' Would easily pass any reasonable code review. It's not that smelly or unusual. It might be unusual with strings, which makes this less common, but it's not an unusual format at all. It's a non-trivial part of the reason ++ operator exists in the first place.
- ehaliewicz2 8y agoIt's very different when you can see the side effect right there, vs happening somewhere else entirely.
- kazinator 8y agoPlus: since Java has ordered evaluation, that tends to encourage side effects in expressions, since they are well defined. (Unless, as we can see, someone doesn't know what they are doing in compiler maintenance, but I mean besides that.)
- dfabulich 8y agoI'd call this code out as unusual at the very least, particularly if this code appeared in a loop. In that case, I'd normally expect to use a for loop, where the ++ operator appears only in the increment part of the loop declaration.
- a1369209993 8y agoUnusual, sure, but I've frequently seen/written code like: if(haveitem) buf[i++] = item; and if(haveitem) line[i++%nline] += sprintf(" %8i",item); seems like an obvious way to evenly distribute column-grouped data.
- watwut 8y agoIt can easily happen when you are iterating through list while updating another array with data from list (conditionally or doing computation or whatever). Pretty common case.
- deleted 8y ago[deleted]
- chrisseaton 8y ago> Would easily pass any reasonable code review. It's not that smelly or unusual. I wouldn't pass that in a code review without comment! I'd ask the author to consider breaking down the two actions into two separate lines so that it's more clear that there are two actions and what the sequential ordering of them is. Inline side effects like that are a mess. Without looking it up, I could not even tell you if the sequence point ordering of that kind of expression in general is defined in C!
- watwut 8y agoIn Java, the sequence of point ordering is remarkably clear and I would expect professional to understand it. I tend t be conservative in syntax and avoid less known parts, but this part is well known.
- deleted 8y ago[deleted]
- chrisseaton 8y agoWhy write code that takes a professional to understand, when you can easily write code that anyone can understand?
- watwut 8y agoYou dont need to be professional to understand that code. It is not obscure what it means. You need someone who knows what ++ operator means and that is usually lesson 1 or 2 in Java for complete beginners class. If you never ever worked with Java and coded in C, then it might be different. Typical Java programmer, even weak one, is fluent in Java 101, but does not really know what might confuse C programmer. In any case, I would expect C programmer finding job as Java programmer for some reason to simply ask someone in office.
- chrisseaton 8y agoIt's not about that specific code - it's about in general having side effects inline in an expression. I can't recall perfectly from the top of my head the rules for the ordering of inline side effects in general in C and Java, and I work in the VM research group at Oracle. Can you? Are you sure about that?
- blaisio 8y agoThat style of code is pretty unusual in Java. Most of the time you would just use a for loop.
- bradleyjg 8y agoOr given that it's java 9 something with streams.
- kllrnohj 8y agoThere's a lot of cases where you can't "just use a for loop" such as if you're looping over something else, or if you're looping over 2 things. For example replace it with something like: data[i++] += other[j++] Yeah you could do this other ways, but it's not actually uncommon. You'll see this sort of thing in collection implementations regularly, for example, or things doing parsing. Not necessarily with the append operator as well, but certainly indexing into an array with a ++ variable.
- chopin 8y agoI've seen and reviewed plenty of Java code and came never across something like this. And I have some dislike for "clever" code. I trade keystrokes for readability every day.
- Roboprog 8y agoWell, here is what I see when I look at that: data[ index ++ ] = data[ index ++ ] + “a”; But that can’t be the right expansion. Ugh. Today’s code sample on Sesame Street, brought to you by the letters “F” & “U”, and the number 2. Why would you subject somebody to something that edgy?
- Roboprog 8y agoAnd yeah, I get that the original statement was supposed to mean: int temp = index ++; data[ temp ] = data[ temp ] + "a"; But the "FP wanna be" guy in me just hates that kind of C inspired foolishness.
- kelnos 8y agoAgreed. I very much dislike "clever" code, but this seems like such a common idiom to me that it wouldn't raise any flags at all during a review.