limiting parameters while reducing lots of ifs

is this the correct way to handle limiting. I have a huge code that has 20 plus places where numbers are limited for example setting month with buttons once you get lower than 1 you want it to go to 12 or higher than 12 go to 1

int screen = 15;
int gap = 200;

int limit(int a, int b, int c, int d, int e) {
  if (a <= b) {
    a = d;
  }
  if (a >= c) {
    a = e;
  }
  return (a);
}

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
}

void loop() {
  // put your main code here, to run repeatedly:

  screen = limit(screen, 1, 12, 8, 2); //parameter, min, max,
  // if lower than min return, if higher than max return
  gap = limit(gap, 25, 255, 25, 255);
  Serial.println (screen);
  Serial.println (gap);
}

it compiles and does what its asked to do but is there a better way

You can use the % operator, but it uses a 0 offset, not 1.

I don’t understand why you have 5 parameters if all you want to do is check whether X is between Y or Z

Also your function would be much easier to use with meaningful names. For example

int limit( int val, int minVal, int maxVal) {
   if (val < minVal) {
      val = minVal;
   }
   if (val > maxVal) {
      val = maxVal
   }
   return val;
}

…R

gpop1:
it compiles and does what its asked to do but is there a better way

If it works, and it makes sense to you, then it is a correct solution. While I don’t see anything intrinsically wrong with your solution, the one thing that stands out to me about it is its lack of self-documentation.

What I mean by that is you have a function, with various parameters that you pass - but only by reading the code can you tell what the function does. I can’t simply look at the function definition and tell what it does from the names of the parameters, nor do I see any comments that tell me that, either. Secondly, even if there were comments, and the variables expressed differently - looking at the calls to the function in the code doesn’t give me any clue as to how it functions.

Why is this important? Well - maintainability for the most part. Should you hand this code to someone else, or should you come back to it after a period of time has passed - the more readable and understandable you make it, the easier it is for someone else or your future self to understand it.

So - let’s try an iterative approach to making it more readable - here’s the original function:

int limit(int a, int b, int c, int d, int e) {
  if (a <= b) {
    a = d;
  }
  if (a >= c) {
    a = e;
  }
  return (a);
}

So - let’s first make it more readable by improving the names of the variables, plus adding some commenting:

/*
 * Apply upper and lower limits to a given value
 *
 * @param value integer value to check
 * @param min integer minimum value can be before limiting
 * @param max integer maximum value can be before limiting
 * @param lower integer value to set for lower limit
 * @param upper integer value to set for upper limit
 *
 * @return int
 */
int limit(int value, int min, int max, int lower, int upper) {
  if (value <= min) {
    value = lower;
  }

  if (value >= max) {
    value = upper;
  }

  return (value);
}

As you can see, by adding a few comments, and changing some variable names, it becomes much more readable.

The comments in the comment block before the function declaration are formatted for Doxygen (http://www.stack.nl/~dimitri/doxygen/), which is almost a defacto standard now for documenting code (if you do all of your code like this, you can run a doxygen translation program over the source code and get an instant set of API documentation for what your code does - which is very, very handy).

The variables are all named in a manner to indicate their use - through the combination of both (and, had the function been more complex, inline comments in the code as well), the code become clearer. You may also notice that I added some spacing to the code; used properly (and consistently I might add!) - these three minor updates can do wonders for the future maintenance of a code base.

More can be done, of course. One thing I noticed was that your code set the limits based on checks using “less-than-or-equal” and “greater-than-or-equal”. I am not sure this is completely correct; usually, when you say “this is the maximum value” and “this is the minimum value” to be accepted, you are saying that anything under or over that will cause a change - not that the value for the min/max is included in that check. To me, that would mean not including the “equals” part of the check:

/*
 * Apply upper and lower limits to a given value
 *
 * @param value integer value to check
 * @param min integer minimum value can be before limiting
 * @param max integer maximum value can be before limiting
 * @param lower integer value to set for lower limit
 * @param upper integer value to set for upper limit
 *
 * @return int
 */
int limit(int value, int min, int max, int lower, int upper) {
  if (value < min) {
    value = lower;
  }

  if (value > max) {
    value = upper;
  }

  return (value);
}

You could also make the code more efficient by returning from the check immediately should one of the checks pass - because if the one check passes, the other should fail - so returning early is better than performing both checks:

/*
 * Apply upper and lower limits to a given value
 *
 * @param value integer value to check
 * @param min integer minimum value can be before limiting
 * @param max integer maximum value can be before limiting
 * @param lower integer value to set for lower limit
 * @param upper integer value to set for upper limit
 *
 * @return int
 */
int limit(int value, int min, int max, int lower, int upper) {
  if (value < min) {
    return lower;
  }

  if (value > max) {
    return upper;
  }

  return value;
}

If you didn’t need to be able to set a different upper or lower value after a bounds check, you could just use the upper and lower value as your returned value:

/*
 * Apply minimum and maximum limits to a given value
 *
 * @param value integer value to check
 * @param min integer minimum to limit value to
 * @param max integer maximum to limit value to
 *
 * @return int
 */
int limit(int value, int min, int max) {
  if (value < min) {
    return min;
  }

  if (value > max) {
    return max;
  }

  return value;
}

You could also break out the min/max checks into their own functions (now we are getting into the territory of creating a “bounds checking function library” - aka “minmax”):

/*
 * Apply minimum limits to a given value
 *
 * @param value integer value to check
 * @param min integer minimum to limit value to
 *
 * @return int
 */
int min(int value, int min) {
  if (value < min) {
    return min;
  }

  return value;
}

/*
 * Apply maximum limits to a given value
 *
 * @param value integer value to check
 * @param max integer maximum to limit value to
 *
 * @return int
 */
int max(int value, int max) {
  if (value > max) {
    return max;
  }

  return value;
}

/*
 * Apply minimum and maximum limits to a given value
 *
 * @param value integer value to check
 * @param min integer minimum to limit value to
 * @param max integer maximum to limit value to
 *
 * @return int
 */
int minmax(int value, int min, int max) {
  return max(min(value, min), max);
}

Note that this example - while providing a couple of extra functions (if the complete min/max check isn’t needed) - does suffer from multiple function calls (with no instant guard-statement return), and is thus a bit less efficient than prior examples. That said, given the simplistic nature of the code, the trade-off is negligible for most use-cases, IMHO.

You could take this further - and turn it into a minmax class; properly doing so would let you set your parameters once in the constructor, then you could simply call the minmax() method with the singular value to check and return; the code would become much cleaner and more readable (at the expense of the added slight complexity of class and object notation - which can be an alien thing at first if you aren’t familiar with it; most of the libraries the Arduino uses, though, are object oriented - so it is something you might want to consider if you haven’t yet done so).

I agree with the names and I changed them before using it.

I am using 5 variables because some of my code requires a roll over rather than a min/max

I was using stuff like the code posted below to handle rolling the number over when setting the clock using key inputs. and as this kind of code is repeated in the code in 20 places so it just seemed better to try a different approach. I was hoping there was a instruction that could do this for me but it seems to work this way.

        if (set_hour < 0) {
          set_hour = 23;
        }
        if (set_hour > 23) {
          set_hour = 0;
        }

maybe I should have done it with 3 variables and made the code twice so one does max to max and the other does max to min.

a lot to take in crosh. I will make some small sketchs and play with the different ideas. thanks for the ideas

Arduino supports the constrain() function

does that not do what you want?

If you are not using functions (which can just return the new value when the if-statement fits) you can always use “else” to prevent unnecessary checks.

if (set_hour < 0) {
  set_hour = 23;
}
if (set_hour > 23) {
  set_hour = 0;
}

Notice how set_hour cannot possibly be < 0 AND > 23, so why are you checking both?

if (set_hour < 0) {
  set_hour = 23;
}
else if (set_hour > 23) { //Else-If, now this will not be checked if set_hour < 0
  set_hour = 0;
}

I knew there had to be a instruction some where that did this. dam google. thanks robtillaart that takes care of the half of them.

gpop1: is this the correct way to handle limiting.

Considering proper user interface design, no. You should not secretly modify user input. If a user enters information that is out of bounds, it is probably not intentional. So if the user enters "31" instead of "1" for hours, they should get an error message and be prompted again for input, instead of the hours being set to 23.

aarg: Considering proper user interface design, no. You should not secretly modify user input. If a user enters information that is out of bounds, it is probably not intentional. So if the user enters "31" instead of "1" for hours, they should get an error message and be prompted again for input, instead of the hours being set to 23.

sry I know how hard it is to understand when I haven't given you any details. True fact is im fixing a code I wrote over 3 months ago with less than I month experience of working with C++. I Only have 3 buttons to change 22 screens and 18 parameters (up-down-edit) so everything has to be limited or rolled over from the up/down buttons.

Codes ran non stop for the last 3 months so its good code wrote really badly and when I went to load it to a uno for testing (it runs on a mega) I ran out of space (128%) so im rewiting some and im now down to (71%) which just proves how bad the original code was.

added the else ps991 thanks

ok code now looks like this after taking in every ones ideas. Its only used on rollovers now as constrain is now used on the basic limits

int rollover(int val, int val_min, int val_max, int new_val_for_min, int new_val_for_max) {
  if (val < val_min) {
  return (new_val_for_min);
  }else if (val > val_max) {
    return (new_val_for_max);
  }
  return (val);
}

You don't have to, but your naming scheme is considered improper. You are "supposed" to use camel case, which just means the first letter is lower case and every word after that starts with an upper case.

So, new_val_for_max "should" be newValForMax because thisIsHowCamelCaseShouldLookAccordingToSomeSmartPeopleWhoLikeToMakeRules

Ps991: You don't have to, but your naming scheme is considered improper. You are "supposed" to use camel case

Who says? Both naming schemes are common and equally valid. There's no "rule" either way.

I personally prefer to use hyphens to separate words in identifiers (in languages that support it) because it's easier to type than either underscores or camel case and is just as easy or easier to read.

and is just as easy or easier to read.

In your opinion. And you are quite alone in sharing that opinion.

PaulS: In your opinion. And you are quite alone in sharing that opinion.

this-is-just-as-easy-to-read-as-underscore-separated-words-but-easier-to-type-than-camel-case.

thisIsNotAsEasyToReadAsUnderscoreSeparatedWordsAndHasTheDistinctDisadvantageInThatItCannotBeWordWrapped.

Capital letters are simply more difficult to read than lowercase letters, and it especially doesn't help to capitalize every word as in camel case. On the other hand, the hyphens in hyphen-separated words "disappear" as familiarity with the naming scheme grows.

Capital letters are simply more difficult to read than lowercase letters

For who? Or, should I ask: for who?

On the other hand, the hyphens in hyphen-separated words "disappear" as familiarity with the naming scheme grows.

In-which-case,-they-are-useless.

Can you even use hyphens in C variables? This looks like variable "hyphenated" minus variable "word", to me:

var = hyphenated-word;

PaulS: In-which-case,-they-are-useless.

They're not useless; they separate words. They "disappear" in the sense that you would eventually see them as spaces.

aarg: Can you even use hyphens in C variables? This looks like variable "hyphenated" minus variable "word", to me:

var = hyphenated-word;

No. I noted that I prefer to hyphenate identifiers in languages that support it (such as Lisp and CSS). C and C++ don't support it.