just practicing for fun on some problem i saw posted on yahoo answers,mathematics section
problem: xyz times pqr = some number
x,y,z,p,q,r should be 2, 3, 5, or 7. and some number's digits should also be made
up entirely of 2, 3, 5, or 7.
solution:
335 x 753 = 252255 or 753 x 335 = 252255
353 x 775 = 273575 or 775 x 353 = 273575
355 x 725 = 257375 or 725 x 355 = 257375
377 x 725 = 273325 or 725 x 377 = 273325


my program starts with x = y = 222. multiplies x times every y from 222 through 777.
then increments x by 1 and loops until x is 777. so the first thing it does is 222 x 222. the last thing it does is 777 x 777.

if x, y, and the product are all composed entirely of 2's, 3's, 5's, or 7's, then it will output to the file.

my code is crude/ugly/ and generally representative of my programming skills.
please help me improve by explaining to me:
How would you write this?
Thanks!

#include <iostream>
#include <cmath>

#include <fstream.h>   // file I/O
#include <iomanip.h>   // format manipulation


using namespace std;

void main()
{
int test;
int i;
int j=1;
int x=222;
int y=222;

  ofstream myfile;
  myfile.open ("alexprimeproducts.txt");



while ( j<556){
for (i=1; i<556; i++){
if (x*y %10  == 2 ||  x*y %10  == 3 ||x*y %10  == 5 ||x*y %10  == 7 )
{
if (x*y/10 %10  == 2 ||  x*y/10 %10  == 3 ||x*y/10 %10  == 5 ||x*y/10 %10  == 7 )
{
 if (x*y/100 %10  == 2 || x*y/100 %10  == 3 ||x*y/100 %10  == 5 ||x*y/100 %10  == 7 )
{
if (x*y/1000 %10  == 2 ||x*y/1000 %10  == 3 ||x*y/1000 %10  == 5 ||x*y/1000 %10  == 7 )
{
if (x*y/10000 %10  == 2||x*y/10000 %10  == 3 ||x*y/10000 %10  == 5 ||x*y/10000 %10  == 7 )
{
 if (x*y/100000 %10 == 2||x*y/100000 %10  == 3 ||x*y/100000 %10  == 5 ||x*y/100000 %10  == 7 )
{
if (x %10 == 2 || x%10 == 3 || x%10 == 5 || x%10 == 7)
{
if (y %10 == 2 || y%10 == 3 || y%10 == 5 || y%10 == 7)
{
 if (x/10 %10 == 2 || x/10%10 == 3 || x/10%10 == 5 || x/10%10 == 7)
{
if (y/10 %10 == 2 || y/10%10 == 3 || y/10%10 == 5 || y/10%10 == 7)
{
if (x/100 %10 == 2 || x/100%10 == 3 || x/100%10 == 5 || x/100%10 == 7)
{
 if (y/100 %10 == 2 || y/100%10 == 3 || y/100%10 == 5 || y/100%10 == 7)
{
//cout << x << "   " << y << "   " << x*y << endl;
myfile << x << " x " << y << " = " << x*y << endl;
}
}
 }
}
}
 }
}
}
 }
}
}
 }
x++;
} //end of first for
x=222;
y++;
j++;
} //end of while

myfile.close();
//cin >> test;

}

1. Learn to format the code better so that you and others can more easily read/understand it. Don't be afraid to make liberal use of spaces and tabs. Aligh each { with matching } and don't put the all at the same place.

2. Recognize repeating code and try to rearrange so that it doesn't repeat. For example if (x*y %10 == 2 || x*y %10 == 3 ||x*y %10 == 5 ||x*y %10 == 7 ) Here you are multiplying x and y four times which takes up a lot of processing time because multiplication and division are pretty expensive. A more efficient way to write that line is:

long n = (x * y) % 10;
switch(x)
{
   case 2: case 3: case 5: case 7: 
      // do something
}

Use of the switch above might be just personal programming style and the if statement would probably work just as well. The main thing is to remove as much repeating code as possible.

Learn to format the code better so that you and others can more easily read/understand it

...but at least he used code tags right??

In your code, you've tested every number from 222 to 777. Most of those don't satify your original statement of only having digits 2,3,5,7 - so don't test them! It'ss a waste of time.

while x*y%10 is repeated, it may be optimised into a single operation by the compiler, so maybe it's not an issue...

using an array of precomputed lookups for the digits makes accessing digit values quite quick.

Because the set of digits E {2,3,5,7} is a power of 2, you can you the bitwise AND operator instead of modulus division for dereferencing the lookups - which I believe is a fair bit quicker.

If the output must be 6 digits long, then you can quickly check that the output is in the range 222222 <= output <= 777777, instead of having to deconstruct the number into digits.

main(){
   int digits[3][4] = {{2,3,5,7}, {20,30,50,70}, {200,300,500,700}};
   
   // there are 4x4x4x4x4x4 = 4096 possible input permutations
   for(int i = 0; i < 4096; i++){
      // extract digits
      int xyz = digits[0][(i&(3<<0))>>0] + digits[1][(i&(3<<2))>>2] + digits[2][(i&(3<<4))>>4];
      int pqr = digits[0][(i&(3<<6))>>6] + digits[1][(i&(3<<8))>>8] + digits[2][(i&(3<<10))>>10];

      // calc product
      int someNum = xyz*pqr;
      int temp = someNum;
      
      // does number have to be 6 digits?
      if(someNum > 777777 || someNum < 222222)
         continue;
      
      // check all digits of result
      bool numOk = true;
      while(temp > 0){
         int digit = temp % 10;
         if(digit != 2 && digit != 3 && digit != 5 && digit != 7){
            numOk = false;      // bad digit
            break;
            }
         temp /= 10;
         }
      
      if(numOk){
         cout << xyz << " x " << pqr << " = " << someNum << endl;
         }
      }
         
   system("pause");
   }

The last code I posted doesn't account for '0' digits in the output - this can be done by changing the while(temp > 0){ to for(int j = 0; j < 6; j++){

wow thank you both for your tips. and dougy... thanks for rewriting the entire program. your idea is much nicer than mine!

This article has been dead for over six months. Start a new discussion instead.