patterncppMinor
Checking for certain values in a range of numbers
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):
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.