0
\$\begingroup\$

I think this code cannot be shortened much, especially because I only consider two cases. But maybe there's a language construct that I don't know that would help.

if (subControl.inverted) {
    newValue = subControl.value+(float)(1.0f/(float)subControl.decorator.ticks);
    if (newValue > 1)
        newValue = 0;
} else {
    newValue = subControl.value-(float)(1.0f/(float)subControl.decorator.ticks);
    if (newValue < 0)
        newValue = 1;
}

Some of the (floats) are gratuitous, which is irrelevant.

Edit: The code could look like this, which helps. However, I guess this is the result of reading the answers already ;)

if (subControl.inverted) {
    newValue = subControl.value+(float)(1.0f/(float)subControl.decorator.ticks);
} else {
    newValue = subControl.value-(float)(1.0f/(float)subControl.decorator.ticks);
}

if (newValue > 1)
    newValue = 0;
if (newValue < 0)
    newValue = 1;
\$\endgroup\$
4
\$\begingroup\$

Not a lot shorter, but a bit...

float ticks = (float)(1.0f/(float)subControl.decorator.ticks);
newValue = subControl.value + subControl.inverted ? ticks : - ticks;

if (subControl.inverted && newValue > 1)  {
    newValue = 0;
} else if (!subControl.inverted && newValue < 0){
    newValue = 1;
}

You could make it even shorter if the you are restricting the values to just 0 and 1.

EDIT: you say in a comment, that it should be between 0 and 1, so...

if (newValue > 1)  {
    newValue = 0;
} else if (newValue < 0){
    newValue = 1;
}

Which is fewer characters if not fewer lines, and imo clearer in intent.

\$\endgroup\$
  • \$\begingroup\$ Really, I might even be able to simplify the last bit, since newValue = MAX(0, newValue) and newValue = MIN(1, newValue) regardless of whether it's inverted or not. I mean, the thing is constrained to 0 to 1 and flips at end of range. So your code is really tight and, thanks to using one more variable, really easy to read. \$\endgroup\$ – Dan Rosenstark Mar 22 '12 at 15:56
2
\$\begingroup\$

If you don't mind using the ternary operator, I suppose you could do something like this:

if (subControl.inverted) {
    newValue = subControl.value+(float)(1.0f/(float)subControl.decorator.ticks);
    newValue = newValue > 1 ? 0 : newValue;
} else {
    newValue = subControl.value-(float)(1.0f/(float)subControl.decorator.ticks);
    newValue = newValue < 0 ? 1 : newValue;
}

Or shorten it further to:

newValue = subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks);
if (subControl.inverted) {
    newValue = newValue > 1 ? 0 : newValue;
} else {
    newValue = newValue < 0 ? 1 : newValue;
}

And if you really want to get funky:

newValue = subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks);
newValue = subControl.inverted ? (newValue > 1 ? 0 : newValue)
                               : (newValue < 0 ? 1 : newValue);

...Not that I think this last version is good - it's a bit harder to follow, and I probably wouldn't use it myself. But it's shorter, which is what you wanted, and I don't know if I can make it any shorter.


If this were golf, here would be the hole-in-one:

newValue = subControl.inverted ? ((subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks))> 1 ? 0 : ((subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks))< 0 ? 1 : (subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks))) );

cleaned up a bit:

newValue = subControl.inverted ?
               (
                  (subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks))> 1
                  ? 0
                  :
                    (
                      (subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks))< 0
                      ? 1
                      :
                        (subControl.value + (subControl.inverted?1:-1) * (float)(1.0f/(float)subControl.decorator.ticks))
                    )
              );

And remember: Just because you can doesn't mean you should!

(I haven't tested this last one, BTW)

\$\endgroup\$
  • \$\begingroup\$ Fascinating, I didn't know (or remember) that the ternary operator is so versatile. The last one isn't SO bad, really. \$\endgroup\$ – Dan Rosenstark Mar 22 '12 at 15:52
  • \$\begingroup\$ As I said down below to @jmoreno, I really don't care whether it's inverted or not in the last part: if it's past one it's always 0 (loops around) and if it's below 0 it's 1 (same). \$\endgroup\$ – Dan Rosenstark Mar 22 '12 at 15:58
  • \$\begingroup\$ @Yar: Would it be correct to say in English "If newValue is positive OR zero, then set it to 1, ELSE (if negative) set it to 1"? \$\endgroup\$ – FrustratedWithFormsDesigner Mar 22 '12 at 16:10
  • \$\begingroup\$ No! The new value could be between zero and one. I just want it to wrap around if it's beyond the limits. \$\endgroup\$ – Dan Rosenstark Mar 22 '12 at 16:16
  • 1
    \$\begingroup\$ @Yar: If it's between 0 and 1, it's still a positive value. Oh wait, I think the English translation should have been: "If newValue is larger than 1, then set it to 0, ELSE (if less than 0) set it to 1" \$\endgroup\$ – FrustratedWithFormsDesigner Mar 22 '12 at 16:26
1
\$\begingroup\$

I think the code you posted (first version) is the most readable version of what you're trying to do, and that's really more important than getting your code smaller.

The only thing I would change is, as jmoreno suggested, use a variable to hold the ticks and then add or subtract depending on the value of inverted. But I would keep the if blocks as you have them in the first version of your code.

\$\endgroup\$
  • \$\begingroup\$ I agree. My code is also more efficient than my second version (in the edit to my question) which would run both if. Obviously it's VERY unlikely that saving an if matters in terms of performance, but it's a point to keep in mind. \$\endgroup\$ – Dan Rosenstark Mar 23 '12 at 14:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.