Cleanup #5233
Cleanup #5233
Conversation
| return (((((light >> 16) & 0xff) + ((dark >> 16) & 0xff)) / 2) << 16) | ||
| | (((((light >> 8) & 0xff) + ((dark >> 8) & 0xff)) / 2) << 8) | ||
| | ((((light) & 0xff) + ((dark) & 0xff)) / 2); | ||
| return ((light >> 16 & 0xff) + (dark >> 16 & 0xff)) / 2 << 16 |
There was a problem hiding this comment.
i don't really see this as an improvement
There was a problem hiding this comment.
5 parenthesis really doesn't help here either, I would even say there are too many to really keep track when reading it, so it causes more confusion than it helps
The pattern of value >> offset & MASK is in my opinion common enough to extract certain bits from a variable to not need additional ().
There was a problem hiding this comment.
Yes it helps, because no, i do have to look up bit shifting operator precedence regularly.
There was a problem hiding this comment.
Same for me, I have no clue about the operator precedence of bitwise operations.
There was a problem hiding this comment.
Neither solution is ideal. Personally I understand the clean one better, even when taking into account that I have to lookup the operator precendence. Mostly because I only have to look it up once and then apply it everywhere. While keeping track of the shitton of parenthesis is actually more challenging for me and is pretty much guaranteed to move one randomly by accident without noticing it.
Otherwise there might be a helper somewhere to calculate the average of two rgb ints?
There was a problem hiding this comment.
I guess int rAverage = (((light >> 16) & 0xff) + ((dark >> 16) & 0xff)) / 2; and so on would work better if we want to clean up that statement.
No description provided.