2

Suppose I want to calculate net_salary of an employee after taking into account the number of years he's worked and the number of kids he has. I don't want to use nested if statements since that will complicate the number of checks I need to make.

double base_salary, net_salary;
int nmbr_kids, nmbr_years;

if(nmbr_kids >= 1 && nmbr_kids <3){
    net_salary = base_salary + 200;
}
else if(nmbr_kids >= 3 && nmbr_kids <4){
    net_salary = base_salary + 400;
}
else if (nmbr_kids >= 4){
    net_salary = base_salary + 600;
}
else{
net_salary = base_salary;
}
/* now I want to account for the number of years worked by the employee and update accordingly his net_salary */

if(nmbr_years >= 1 && nmbr_years <3){
    net_salary = net_salary + 200;
}
else if(nmbr_years >= 3 && nmbr_years <4){
    net_salary = net_salary +400;
}
else if(nmbr_years >= 4){
    net_salary = net_salary + 600;
}
else{
net_salary = net_salary;
}

Is there a better, more compact way to do the above? Or am I looking at the problem the wrong way?

idnow
  • 27
  • 2
  • 5
    Where do you get paid more just for having kids? :) – user14717 Jan 03 '15 at 20:01
  • Apart from properly indenting the code, adding spaces around `=` and putting `}` on the same line as `else`, I don't think you can do much to improve readability. In this particular case, you could maybe get away with some cleverly constructed switch, but it can be even harder to read. – MightyPork Jan 03 '15 at 20:01
  • 1
    @idnow: you could replace `if(nmbr_kids >= 1 && nmbr_kids <3)` with `if(nmbr_kids == 1 || nmbr_kids ==2)`, and this: `if(nmbr_kids >= 3 && nmbr_kids <4)`, with this: `if(nmbr_kids ==3)` – Giorgi Moniava Jan 03 '15 at 20:18

3 Answers3

5

The first sequence can be altered to:

if (nmbr_kids == 0)
    net_salary = base_salary +   0;
else if (nmbr_kids < 3)
    net_salary = base_salary + 200;
else if (nmbr_kids < 4)
    net_salary = base_salary + 400;
else 
    net_salary = base_salary + 600;

The key idea here is to eliminate conditions. The original had to check both ends of the range because the case of 0 kids was not dealt with first.

Alternatively, you can define an array:

double kids_bonus[] = { 0, 200, 200, 400, 600, 600, 600, 600 };
enum { NUM_KIDS_BONUS_VALUES = sizeof(kids_bonus) / sizeof(kids_bonus[0]) };

int eff_kids = nmbr_kids;
if (eff_kids >= NUM_KIDS_BONUS_VALUES)
    eff_kids = NUM_KIDS_BONUS_VALUES - 1;
net_salary = base_salary + kids_bonus[eff_kids];

Or, with a min function:

static inline int min(int x, int y) { return (x < y) ? x : y; }

int eff_kids = min(nmbr_kids, NUM_KIDS_BONUS_VALUES - 1);
net_salary = base_salary + kids_bonus[eff_kids];

Of course, if you have an upper limit on the number of kids, you can simply create an array big enough.

Your original condition else if (nmbr_kids >= 3 && nmbr_kids < 4) would be better written as else if (nmbr_kids == 3), of course.

The same tactics can be used in the second case too.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
1

You could do the following:

int find_value(int number, int * array, int length) {
    if (number < length) {
        return array[number];
    } else {
        return array[length-1];
    }
}

#define LEN(arr) (sizeof(arr)/sizeof(arr[0]))
#define FIND(number, array) find_value(number, array, LEN(array))

int add_kids[] = {0, 200, 200, 400, 600};
int add_years[] = {0, 200, 200, 400, 600};

// ...

net_salary = base_salary + FIND(nmbr_kids, add_kids) 
                         + FIND(nmbr_years, add_years);

This is called a lookup table.

glglgl
  • 89,107
  • 13
  • 149
  • 217
  • In the function, you could use: `if (number >= length) number = length - 1; return array[number];`. You and I both assumed that there are no negative kids or years service — people have a bad habit of encoding information in such fields, but without information to that effect, it is reasonable to assume non-negative values. – Jonathan Leffler Jan 03 '15 at 20:22
1

Using the ternary conditional operator you could write it as:

net_salary = base_salary +
    nmbr_kids == 0 ? 0 :
    nmbr_kids <= 2 ? 200 :
    nmbr_kids <= 3 ? 400 : 600;

net_salary +=
    nmbr_years == 0 ? 0 :
    nmbr_years <= 2 ? 200 :
    nmbr_years <= 3 ? 400 : 600;

Or keeping the original logic in the question the first part would be:

net_salary = base_salary +
    nmbr_kids >= 1 && nmbr_kids < 3 ? 200 :
    nmbr_kids >= 3 && nmbr_kids < 4 ? 400 :
    nmbr_kids >= 4 ? 400 : 0;
Forss
  • 778
  • 1
  • 4
  • 13