HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Checking for certain values in a range of numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rangenumberscheckingforvaluescertain

Problem

Is there any chance to make this script run faster?The script goes through each number from a to b, and checks if it holds 1, if it holds 2, if it holds 3, if it holds 1 and 2, if it holds 1 and 3 and if it holds 1, 2 and 3. Then it saves all results.

unsigned long long a, b, f;

    a = 1;
    b = 100000000;
    int f1 = 0, f2 = 0, f3 = 0, x1 = 0, x2 = 0, x3 = 0, x4 = 0, x5 = 0, x6 = 0, x7 = 0;   
    for(int i = a; i  0) {
                x1++;
            }
            if(f2 > 0) {
                x2++;
            }
            if(f3 > 0) {
                x3++;
            }
            if(f1 > 0 && f2 > 0) {
                x4++;
            }
            if(f1 > 0 && f3 > 0) {
                x5++;
            }
            if(f2 > 0 && f3 > 0) {
                x6++;
            }
            if(f1 > 0 && f2 > 0 && f3 > 0) {
                x7++;
            }
            f1 = 0;
            f2 = 0;
            f3 = 0;
        }

Solution

First, I would tell you to restructure the while into a do-while. This will allow you to run at least one iteration of your loop (for one-digit numbers), THEN see if you need to repeat. That will in turn allow you to avoid the additional double-checks to make sure you weren't ignoring one-digit numbers (which are currently performed unnecessarily on any number > 10 that honestly doesn't have a 1, 2, or 3 in it).

Other than that, I would suggest you break early. Use booleans instead of ints: once you know you have a 1, 2, or 3, don't keep checking digits for those values (meaning if you have all three you can exit the loop entirely; for 1000000000000000123, you're done looping through the divide/modulo loop after checking just the first three digits).

Lastly, you're making checks you already know are false. You're first checking if f1 > 0, then regardless of that result you're checking if f1 and f2 are > 0. Instead, try nesting ifs; once you know f1 > 0 you can check if f2 is also > 0, while if f1==0 you don't have to check if f1 > 0 and f2 > 0 because you already know it can't be true. Similarly, if any digit of f is 1, then the same digit cannot be 2 or 3, so as soon as you know what a digit is, continue to the next digit.

Try this (you may have to define bool; older C++ specs do not have it built-in):

unsigned long long a, b, f;

a = 1;
b = 100000000;
bool[] z = new bool[3];
int[] x = new int[7];
for(int i = a; i  3 || c==0) { } 
            //once you know the value (or know the value exists), stop testing
            else if(!z[0] && c == 1) {
                z[0] = true;
            }
            else if(!z[1] && c == 2) {
                z[1] = true;
            }
            else if(!z[2] && c == 3) {
                z[2] = true;
            }

            //this MAY be faster depending on proc architecture and compiler;
            //some CPUs have "arithmetic-in-place" instructions.
            f /= 10;                
        } while(f != 0 && !(!z[0] && !z[1] && !z[2]));

        if(z[0]) {
            x[0]++;
            if(z[1]) { //1&2
               x[3]++;
               if(z[2]) //1&2&3
                  x[6]++;
            }
            else if(z[2]) x[4]++; //1&3
        }
        if(z[1]) {
            x[1]++;
            //we have already checked 1&2 and 1&2&3
            if(z[2]) x[5]++; //2&3                 
        }
        if(z[2] > 0) {
            x[2]++;
            //we have already checked 1&3, 2&3 & 1&2&3
        }

        //this is for conciseness and may not work depending on your C++ spec
        z[0] = z[1] = z[2] = false;
    }

Code Snippets

unsigned long long a, b, f;

a = 1;
b = 100000000;
bool[] z = new bool[3];
int[] x = new int[7];
for(int i = a; i <= b; i++) {
        f = i;
        do{
            var c = f % 10;
            // test the most likely thing first; most digits are NOT 1-3
            if(c > 3 || c==0) { } 
            //once you know the value (or know the value exists), stop testing
            else if(!z[0] && c == 1) {
                z[0] = true;
            }
            else if(!z[1] && c == 2) {
                z[1] = true;
            }
            else if(!z[2] && c == 3) {
                z[2] = true;
            }

            //this MAY be faster depending on proc architecture and compiler;
            //some CPUs have "arithmetic-in-place" instructions.
            f /= 10;                
        } while(f != 0 && !(!z[0] && !z[1] && !z[2]));

        if(z[0]) {
            x[0]++;
            if(z[1]) { //1&2
               x[3]++;
               if(z[2]) //1&2&3
                  x[6]++;
            }
            else if(z[2]) x[4]++; //1&3
        }
        if(z[1]) {
            x[1]++;
            //we have already checked 1&2 and 1&2&3
            if(z[2]) x[5]++; //2&3                 
        }
        if(z[2] > 0) {
            x[2]++;
            //we have already checked 1&3, 2&3 & 1&2&3
        }

        //this is for conciseness and may not work depending on your C++ spec
        z[0] = z[1] = z[2] = false;
    }

Context

StackExchange Code Review Q#9336, answer score: 3

Revisions (0)

No revisions yet.