Any idea how to improve this code?

Any idea what I can do to improve this? I don't want to hand type every possible combination.
1
2
3
4
5
6
  if (storeArguments[0] == 1 && storeArguments[1] == 0)
            print(toPrint, storeArguments[0]);
        if (storeArguments[0] == 0 && storeArguments[1] == 2)
            print(toPrint, storeArguments[1]);
        if (storeArguments[0] == 1 && storeArguments[1] == 2)
            print(toPrint, storeArguments[0], storeArguments[1]);


or to simplify... :
1
2
3
4
5
6
7
8
9
10
 if a is true and b is false
     foo(a)

 if a is false and b is true
     foo(b)

 if a is true and b is true
     foo (a and b)

 // for INFINITE elements 
Since each statement does something different, there is not much simplification you do other than nesting some of the if statements but that will still be the same code. Maybe if you post the other combinations here or describe the problem, someone might be able to find a pattern which uses less code
Make a loop instead of if statements. Or if statements inside a loop.

Although, I am slightly confused by your code above. How do you determine what is true and false? I thought you were determining this by using 1's and 0's but then you used 2.

So are 2's and 1's always true and 0's always false? Are there additional combinations?
anything other than 0 is true, though 2 is not the same as 1, they have specific purposes that matter to the function that gets called. the only thing that will be added is going to be more elements. so instead of comparing 2 elements( a and b) i will have to compare more ( a, b , c , d , e .. .) which will get tedious.
Last edited on
Okay, but then the algorithm will change. Because we only know what a and b are supposed to be. How do the additional elements fit into the algorithm?

1
2
if a is true and b is false and c is ?? and d is ??
     foo(a and ??)


Are you sure when it states "// for INFINITE elements" it didn't mean the values of the variables and not adding additional variables themselves?
here is a way (however I don't recommend it because it will make the code less readable and it is a limited way:
and it <b>requires</b> that for example storeArguments[1]==0 or 2
if we have something like:
1
2
3
4
if(a==1 && b==0)
    foo();
else if(a==2 && b==0)
    bar();

it won't work
1
2
3
4
if (storeArguments[0] ^ storeArguments[1])    //^:XOR
    print(toPrint, storeArguments[0]+storeArguments[1]);   //it will add the item to 0 which gives the item
elseif (storeArguments[0] && storeArguments[1])    //both are non 0
    print(toPrint, storeArguments[0], storeArguments[1]);
Last edited on
@unsensible:
Yes, that is what the code will look like when adding multiple elements, it will just be a whole lot of comparing every element . . .

@JewelCpp:
I'll look into your method, it seems like what I needed!

And, each element will have either 0 as a value or an individual number specific to each one... for example storeArguments[0] will have either 0 or 1
, storeArguments[1] will have either 0 or 2, and so on... So, I don't think what you mentioned would be a problem.
Why not simply Xor 'a' and 'b'? (for the simplified version)

Or '&' them and then compare the results with 0?(for the comprehensive version)

As an additional note, switch statements are faster than multiple "if's", I assume
Last edited on
Use a lookup table?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
#include <functional>
#include <array>
#include <map>

enum op { toPrint /* ... */ };
void print( op, int) { /* ... */ }
void print( op, int,int) { /* ... */ }
void print( op, int,int,int) { /* ... */ }

void foo( std::array<int,4> storeArguments )
{
    static const std::map< std::array<int,4>, std::function< void() > > functions
    {
        { { 1, 0, 0, 0 }, []() { print( toPrint, 1 ); } },
        { { 0, 2, 0, 0 }, []() { print( toPrint, 2 ); } },
        { { 1, 2, 0, 0 }, []() { print( toPrint, 1, 2 ); } },
        { { 0, 0, 4, 0 }, []() { print( toPrint, 4 ); } },
        { { 1, 2, 4, 0 }, []() { print( toPrint, 1, 2, 4 ); } },
        { { 1, 0, 0, 8 }, []() { print( toPrint, 1, 8 ); } }
        // etc.
    };

    auto iter =  functions.find( storeArguments ) ;
    if( iter != functions.end() ) iter->second() ;
}

I'm not sure I 100% understand what that lookup table does, but it seems that it doesnt fix my problem of writing too much code by hand... I was wondering if it could be done programmatically?
Pseudo:

1
2
3
4
5
6
7
8
9
10
11
12
13
FOR j = 1 TO len(storeArguements) - 1 DO:

    BEGIN
    IF (storeArguements[j] && storeArguements[j - 1]):
        print(toPrint, storeArguments[j-1], storeArguments[j]);

    ELIF (storeArguements[j]):
        print(toPrint, storeArguments[j]);

    ELIF (storeArguements[j - 1]):
        print(toPrint, storeArguments[j - 1]);

END FOR
> I was wondering if it could be done programmatically?

It could be done programmatically, if a pattern can be established.

For instance, trivially, if the logic is: call print() with the non-zero elements of storeArguments (in order).

1
2
3
4
5
6
7
8
9
10
11
12
#include <vector>

enum op { toPrint /* ... */ };

void print( op, std::vector<int> args ) { /* whatever */ }

void foo( const std::vector<int>& storeArguments )
{
    std::vector<int> non_zeroes ;
    for( int v : storeArguments ) if(v) non_zeroes.push_back(v) ;
    print( toPrint, non_zeroes ) ;
}

Topic archived. No new replies allowed.