How to remove multiple if else to handle data easily [closed]

Question

I am doing a project which has a custom made board based on PIC32 MCU. It has 30 Inputs and 10 Output. I have to monitor inputs and send there information to UART. Each inputs is defined a code. For example: if input 1 is ON,then data to be send is 11 {'1'--> Input 1, '1' --> ON}. If input 1 is OFF, then data to be send is 10 {'1'--> Input 1, '0' --> OFF}. Similarly is 24th input is on, then 241, if its off, then 240 and same goes for all the inputs.

Now the final data to be send over UART will contain information about all the inputs:

10 20 30 41 50 61.....271 280 291 300 

As I am beginner in programming, what I have done to achieve above syntax is I have defined variables for every inputs.

char input1ON[]  = "11";
char input1OFF[] = "10";
bool input1Stat  =  false;

char input2ON[]  = "21";
char input2OFF[] = "20";
bool input2Stat  = false;

and same for complete 30 inputs. Now in main function:

if(PORTDbits.RD6 == 1)  // if input1 is high
{
  input1Stat = true; //make input1 status true
}
if(PORTDbits.RD6 == 0) //if input1 is low
{
  input1Stat = false; //make input1 status false
}
if(PORTDbits.RD7 == 1)  // if input2 is high
{
  input2Stat = true; //make input2 status true
}
if(PORTDbits.RD7 == 0) //if input2 is low
{
  input2Stat = false; //make input2 status false
}
/*
* same goes for rest of the inputs
*/

then after all the if statements, I am checking if input1stat is true then strcat it, like below:

if(input1Stat == true) //if input 1 status is true
{
  strcat(inputs,input1ON); //concatenate input1ON variable with inputs variable 
}
else
  strcat(inputs,input1OFF); //concatenate input1OFF variable with inputs variable 

if(input2Stat == true) //if input 2 status is true
{
  strcat(inputs,input2ON); //concatenate input2ON variable with inputs variable 
}
else
  strcat(inputs,input2OFF); //concatenate input2OFF variable with inputs variable 

So at the end I am just putting inputs to UART:

putsUART1(inputs);

so as a result I am getting the above string of data on UART. But the problem is there are many if else statements in my code which is making the code unreadable. I think there may be some other way of handling this. As I am beginner, I don't know how to remove all these if else statements. Any one can just give a demo to remove all the if else statements. Thanks.

EDIT

Input 1 and Input 2 is nothing but PORTDbits.RD5 & PORTDbits.RD6. I used Input 1 and Input 2 just for demo. I have made the changes.

Answer 1

When dealing with a set of inputs that are all individual pins that might have arbitrary mappings to physical chip pins, the first thing I do is create a function that maps from logical input number to the physical pin:

bool pin_read (int n)
{
  switch (n) {
  case 0: return PORTDbits.RD6;
  case 1: return PORTDbits.RD7;
  /* ... etc. */
  }
}

Note that such a function would normally be one function in a separately-compiled pin module that also contains functions such as pin_write() and pin_direction(). I use a <module>_<method>() naming convention for my C functions, which really helps with keeping track of things as the project grows.

Once you have such a function, everything else can be handled with loops. To capture the status of all of the pins:

for (i=0; i<N_INPUTS; ++i) {
  inputStat[i] = pin_read(i);
}

To print out the previously-captured status:

for (i=0; i<N_INPUTS; ++i) {
  sprintf (temp_buffer, " %d%d", i, inputStat[i]);
  putsUART1(temp_buffer);
}
putsUART1("\n");

Answer 2

You shouldn't even need if statements at all.

input2Stat = input2;

Will assign the value if false (0) or true (anything not 0)

In general though, most of your clutter is from redundant comments, and line breaks. You don't need the same comment on each section if the code is simple to read, or the comment can apply to all of it, just make that clear.

You can compact the if statement to one line if there is only one instruction it follows, by omitting the curly brackets. Same for else.

 if (input2 == true) input2Stat = true;
 else input2Stat = false;

Answer 3

Assuming you're sampling the input and sending at the same time on a small micro I normally wouldn't bother concatenting the output string, I'd send it straight out the UART on the fly. Maybe you could write a function like this:

void send_in_state(uint8_t io_number, bool val)
{
    if (io_number >= 10)
        putcUART1(io_number / 10 + '0');
    putcUART1(io_number % 10 + '0');
    if (val)
        putcUART1('1');
    else
        putcUART1('0');
    putcUART1(' ');
}

I've just used putcUART1 as a placeholder for function to send a single character, it's probably called something else. Then in the main code you'd have something like:

send_in_state(1, PORTDbits.RD6);
send_in_state(2, PORTDbits.RD7);
// etc...

If you did want to sample at an earlier point before sending you could still use the above function but save the input states earlier:

bool save_input1 = PORTDbits.RD6;
bool save_input2 = PORTDbits.RD7;
// ...
send_in_state(1, save_input1);
send_in_state(2, save_input2);

Answer 4

For this construct:

if(input1 == true)  // if input1 is high
{
  input1Stat = true; //make input1 status true
}
if(input1 == false) //if input1 is low
{
  input1Stat = false; //make input1 status false
}

Assuming that input1 can be only true or false and no third state, you can simply write:

input1Stat = input1 ? 1 : 0;
input2Stat = input2 ? 1 : 0;
...

This is already much easier on the eye but will likely compile down to the same code. I'll explain later why I'm using 1/0 instead of true/false.

if(input1Stat == true) //if input 1 status is true
{
  strcat(inputs,input1ON); //concatenate input1ON variable with inputs    variable 
}
else
  strcat(inputs,input1OFF); //concatenate input1OFF variable with inputs variable

Again, there are only two states. This can be replaced with:

const char *input1Stat[2] = { "10", "11" };
const char *input2Stat[2] = { "20", "21" };
....
strcat( inputs, input1msg[input1Stat] );
strcat( inputs, input2msg[input2Stat] );

Because input1Stat is now 0 or 1, it will select one of the two entries in the array of string pointers.

As a comment to another answer pointed out, in a larger system with "real" pointers and such, you would implement this with array of pointers. In a microcontroller like the PIC, accesses like that can turn in to a lot of code, and I often refrain from hiding the code too much.

If you don't want to repeat yourself, you can handle each input in one place, put it in a macro, then write out the macro as many times as you need.

Answer 5

Create a bit encoded word from your inputs. Something like:

unsigned int encodedinput = input1 ? 1 : 0 +
                            input2 ? 2 : 0 +
                            input3 ? 4 : 0 +
                            input3 ? 8 : 0 + ...etc

Then you can use a loop to go through the individual bits converting them to strings

for (i = 0 ; i < nobits ; i++)
{
 sprintf (tempbuffer, "%d", i) ;
 strcat (str, tempbuffer) ;
 sprintf (tempbuffer, "%d", (encodedinput >> i) & 1) ;
 strcat (str, tempbuffer) ;
}

Or, just simply send the bit encoded word :-)

Answer 6

Well firstly this:

if(input1 == true)  // if input1 is high
{
  input1Stat = true; //make input1 status true
}
if(input1 == false) //if input1 is low
{
  input1Stat = false; //make input1 status false
}

Has exactly the same effect as this:

input1Stat = input1;

So that will save you some space. In fact, you might not need the variable input1Stat at all. Perhaps you can use input1 directly in the other code block? The only difference is exactly when you measure the pin's value.

---

Secondly, you don't need separate codes for each state of every pin. You can construct them automatically from the pin number and 0 or 1 for off or on. So you can do something like

// For pin one
strcat(inputs, "1");
if(input1Stat == true) 
{
  strcat(inputs,ON); 
}
else
{
  strcat(inputs,OFF);
}
strcat(inputs," ");
// and the same for the other pins

Where you've defined ON and OFF as "0" and "1".

---

Thirdly, anytime you are copying out almost the same code for each pin, you're going to be better off looping over those pins. The only tricky thing there is turning the pin number from an char to a string, and putting the pin values somewhere you can access them by number. The function sprintf is designed to do int-to-string stuff and much more, but it's a complicated thing with a lot of overhead... so maybe not the best choice for a PIC. See if the function itoa is available in your environment (it might depend on your complier). If it is, then you can write

bool inputStats[] = {input1, input2 ...};

for (char pin=1; pin<30; pin++)
{
    char buffer[3];
    itoa(pin, buffer, 10);
    strcat(inputs, buffer);
    if(inputStats[pin-1] == true) 
    {
      strcat(inputs,ON); 
    }
    else
    {
      strcat(inputs,OFF);
    }
    strcat(inputs," ");
}