HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Counting from 1 to 3

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromcountingstackoverflow

Problem

Please critique to help me learn:

int getNextInt(int* number)
{
    int value = *number;
    (*number)++;
    if (*number > 3)*number = 1;
    return value;
}


I want a function that will increment an external number by one, when it goes over than 3, then reset it to 1.

essentially when I repeatedly call this function I want the out to me 1,2,3,1,2,3,1,2,3

Solution

Style issue: Formatting and braces
Even if the language allows you to leave out the braces after an if, do not omit them. People tend to expand their code and forget to put them in when required. This creates nasty, hard to find bugs and the compiler will not complain.

Design issue: out parameters Output parameters tend to obscure the information flow in your program. They are not required in any way, you can do everything fine without them. Failing to keep track of all the funny little things a function might modify as a side effect is a constant source of bugs. Many people thus prefer to not use them at all and rather return a compound of all the things the function calculates and distribute them to the data structures after the function has returned. If this becomes hard you may have an overall design issue. (There are still valid reasons for using out-parameters e.g. in performance and memory critical low-level applications.)

Legacy code: Raw pointers Using plain raw pointers is only fun for very small and quick hacks, that will never see any real use. Become friends with the C++11 (and above) smart pointers. Seriously, they will save you a lot of trouble.

Last but not least

if(foo > max){
   foo = min;
}


Is in your case (continuous counting with clamped upper and lower value) equal to

foo = (foo % (max + 1 - min)) + min; // parenthesis for ease of reading


With max = 3 and min = 1 you get

foo = (foo % 3) + 1;


Using the modulo operator is an elegant approach for such "around the clock" counting.

Edit: The modulo operator is now named correctly. (Credits to @Cody Gray)

Code Snippets

if(foo > max){
   foo = min;
}
foo = (foo % (max + 1 - min)) + min; // parenthesis for ease of reading
foo = (foo % 3) + 1;

Context

StackExchange Code Review Q#152405, answer score: 5

Revisions (0)

No revisions yet.