How to make this C code shorter

Question

I have some code that toggles values in a struct.

For example, I find myself writing code a lot that looks like this:

if(options.test == 1)
{
    options.test = 2;
}
else if (options.test == 2)
{
    options.test = 1;
}

Is there a more compact version I can use to make this code shorter?

You are using the comparsion operator '==' inside the brackets. I guess you want an assignment '='? — Benjamin Maurer, Mar 10 '14 at 11:57
If you have only 2 states, you can use an int like a bool instead and just use 'value' and '!value'. The other solution would be a switch statement. — Benjamin Maurer, Mar 10 '14 at 11:59
Don't sacrifice readability, don't use magic numbers, and don't repeat yourself. Consider writing a function to do this check-and-invert instead. — DCoder, Mar 10 '14 at 12:00
possible duplicate of [Run A and then B or run C](http://stackoverflow.com/questions/21627707/run-a-and-then-b-or-run-c) — alk, Mar 10 '14 at 12:12
A very short (& even less readable) code: `options.test ^= 3;` — anishsane, Mar 10 '14 at 12:16

barak manos · Answer 1 · 2014-03-10T12:20:00.960

8

If the values of options.test are either 1 or 2 throughout the execution of your program, then you can simply do:

options.test = 3-options.test;

If this variable may be set to other values, then the best way to handle it is usually with:

switch (options.test)
{
    case 1:   options.test = 2;   break;
    case 2:   options.test = 1;   break;
    case ...: options.test = ...; break;
    case ...: options.test = ...; break;
    case ...: options.test = ...; break;
    default:  options.test = ...; break;
}

If the values are between 0 and N (with a relatively small N), then you may also consider hashing.

For example, instead of:

switch (options.test)
{
    case 0: options.test = 4; break;
    case 1: options.test = 2; break;
    case 2: options.test = 1; break;
    case 3: options.test = 3; break;
    case 4: options.test = 5; break;
    case 5: options.test = 0; break;
}

You can do:

static int hash[] = {4,2,1,3,5,0};
options.test = hash[options.test];

edited Mar 10 '14 at 12:20

answered Mar 10 '14 at 11:58

barak manos

29,648
10
62
114

8

I thought about suggesting this too, but boy would it make the code harder to read and maintain. – Joe White Mar 10 '14 at 11:59
@Joe White, well - OP asks for a "shorter" way, right? (though just as you, I'm not really sure what would that gain)... – barak manos Mar 10 '14 at 12:03
@barakmanos : yours code is indeedly shorter, but the OP didn't wrote in detail if his values are only 1 or 2. If it's 1 - 2, then yours answer is the way to go. – KarelG Mar 10 '14 at 12:04
@KarelG, My answer says (and I quote): "If the values of options.test are either 1 or 2 throughout the execution of your program". – barak manos Mar 10 '14 at 12:05
In this example my values are only 1 and 2. However, other code I have written uses more than just these two values. I was asking if there is a simple way to shorten it to make it easier to read. – user9993 Mar 10 '14 at 12:05
@user9993: If you have other values, then the best way would probably be to use a `switch/case` statement. – barak manos Mar 10 '14 at 12:06

score 2 · Answer 2 · answered Mar 10 '14 at 12:01

2

i would use a switch case block inside a method if you have multiple option values.

options.test = setOptionTestValue(options.test);

method

int setOptionTestValue(value) {
    switch (value) {
        case 1: return 2;
        case 2: return 1;
        default: return 0;
    }
}

answered Mar 10 '14 at 12:01

KarelG

5,176
4
33
49

Good answer, although this fails a bit with the *shorter* criteria that OP asks about – Natan Streppel Mar 10 '14 at 12:04

score 1 · Answer 3 · answered Mar 10 '14 at 12:01

1

It sounds like this is a fairly generic question rather than an exact example. I find that for these simple single line statements dropping the braces is still perfectly readable:

if(options.test == 1)
    options.test = 2;
else if (options.test == 2)
    options.test = 1;

answered Mar 10 '14 at 12:01

PeterJ

3,705
28
51
71

1

@AxitSoni, where did OP ask for one line of code? The question was for *shorter* code. This totally qualifies. – Joe White Mar 10 '14 at 12:06
sorry he edited. may be .. at first when i saw this question at that time it is there :) – toxic_boi_8041 Mar 10 '14 at 12:08

score 0 · Answer 4 · answered Mar 10 '14 at 12:02

You should use 'switch' because it's more beautiful:

switch (options.test) {
   case 1:
      options.test = 2;
      break;
   case 2:
      options.test = 1;
      break;
}

If you have just this code and nothing more, use this:

options.test = 3 - options.test;

score 0 · Answer 5 · answered Mar 10 '14 at 12:05

This looks like it might be a state machine, especially since you said you've been writing this code repeatedly (once for each type of state transition?). What about making a function that returns the new state?

int getNewStateAfterLogin(int oldState) {
    switch (oldState) {
        case 1:
            return 2;
        case 2:
            return 1;
        default:
            return oldState;
    }
}

options.test = getNewStateAfterLogin(options.test);

Of course, if it really is a state machine, you'd want to use an enum, or symbolic constants at the very least. But this gives you a chance to pull the logic into a separate function, and give that function a name.

score 0 · Answer 6 · answered Mar 10 '14 at 12:06

0

For input {1,2}:

int map[3]  = {0, 2, 1};

options.test = map[options.test];

Adjust as needed for other/more input.

answered Mar 10 '14 at 12:06

alk

69,737
10
105
255

How to make this C code shorter

6 Answers6