Arduino driver for 7 segment display with bit-shift register

This code counts down from 30 to -999 on a 7-segment 4-digit display; it works but I don't know if it is a good way to do it or if it follows best practices. I want to start writing libraries for components that are not specifically made for Arduino. This is my first attempt. Please tell me if there are any problems with it or if there is some way for me to improve it. I am working on also making it able to show decimals but am not done yet.

```
//This is code to put out any signed integer to a seven segment display using a bit shift register.

int number[12] = {80,95,50,22,29,148,144,94,16,28,191,16}; //Values 0 through 9 on a 7-segment LED display, negative symbol is 10 and subtract 11 from any number to put a decimal
int shiftPins[3] = {7,8,9}; // (clock pin, data pin, latch pin)
int digitPins[4] = {2,3,4,6}; // (ones, tens, hundreds, thousands)
int show[4]; // (thousands, hundreds, tens, ones)

void setup(){
pinMode(digitPins[0], OUTPUT);
pinMode(digitPins[1], OUTPUT);
pinMode(digitPins[2], OUTPUT);
pinMode(digitPins[3], OUTPUT);

pinMode(shiftPins[0], OUTPUT);
pinMode(shiftPins[1], OUTPUT);
pinMode(shiftPins[2], OUTPUT);
}

void loop(){

print2LEDs(30-millis()/1000);

}

void print2LEDs(int number2show){

if(number2show >= 0){

show[0] = ((int)number2show/1000);
show[1] = ((int)number2show%1000)/100;
show[3] = (((int)number2show%1000)%100)/10;
show[2] = ((((int)number2show%1000)%100)%10);
}
else {
number2show = -(number2show);
show[0] = 10;
show[1] = ((int)number2show%1000)/100;
show[3] = (((int)number2show%1000)%100)/10;
show[2] = ((((int)number2show%1000)%100)%10);
}
showDigit(show[0],digitPins[0]);
showDigit(show[1],digitPins[1]);
showDigit(show[2],digitPins[2]);
showDigit(show[3],digitPins[3]);
}

void showDigit(int i, int f){

digitalWrite(shiftPins[2], 0);
digitalWrite(f, 1);

shiftOut(shiftPins[1], shiftPins[0], LSBFIRST, number[i]);

digitalWrite(f, 0);
digitalWri

What's really in show[2]?

According to your comment:

int show[4]; // (thousands, hundreds, tens, ones)

the digits in show are in descending order. But according to your code:

show[0] = ((int)number2show/1000);
show[1] = ((int)number2show%1000)/100;
show[3] = (((int)number2show%1000)%100)/10;
show[2] = ((((int)number2show%1000)%100)%10);

you swapped show[2] and show[3]. Was this a copy paste error?

Simplification

Your code for setting the show array can be simplified.

• There's no need for an int cast because everything is already an int.

• (((n % 1000) % 100) % 10) is equivalent to n % 10

• Three of the four lines in the if and else case are common.

I would rewrite it like this:

if (number2show >= 0) {
        show[0] = (number2show / 1000) % 10;
    } else {
        show[0] = MINUS_SIGN;
        number2show = -number2show;
    }
    show[1] = (number2show / 100) % 10;
    show[2] = (number2show /  10) % 10;
    show[3] = number2show % 10;
    for (int i=0; i < 4; i++)
        showDigit(show[i], digitPins[i]);

Variable scopes

I'm not sure if you use show anywhere else, but if you don't then you should make it be a local variable instead of a global. Also, your other global arrays can be marked const` since they are read-only.

StackExchange Code Review Q#94389, answer score: 7