Any idea how to improve this code?

Feb 17, 2014 at 8:53pm
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 
Feb 17, 2014 at 9:03pm
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
Feb 17, 2014 at 9:09pm
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?
Feb 17, 2014 at 9:14pm
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 Feb 17, 2014 at 9:15pm
Feb 17, 2014 at 9:28pm
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?
Feb 17, 2014 at 9:45pm
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 Feb 17, 2014 at 9:47pm
Feb 18, 2014 at 6:45am
@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.
Feb 18, 2014 at 6:49am
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 Feb 18, 2014 at 6:55am
Feb 18, 2014 at 7:26am
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() ;
}

Feb 18, 2014 at 2:42pm
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?
Feb 18, 2014 at 2:58pm
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
Feb 18, 2014 at 3:01pm
> 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.