#### Archived

This topic is now archived and is closed to further replies.

# An annoying problem

This topic is 5039 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

I've been working on a program to find factors of a number and compare it with another, and the first part (finding the factors) worked pretty well. Now, I'm trying to store them, and I have the following code:
#include <iostream>
#include <cstdio>
#include <cstdlib>
using namespace std;

int *divide(int it, int *keeper);

int main()      //makes a divider program

{
int divident;
int *list;
int *listo;
cout << "This program divides a number by several others and shows the results.\n What shall the number be?";
cin >> divident;
cout << "\n";
divide(divident, list);
cout << list[1]; // to make sure it works

system("PAUSE");
return 0;
}

int divide(int it, int *keeper)
{
int divider, i;
for (divider=1; divider <= it; divider++)
{
if (!(it % divider))// this was suggested by leiavoia.

keeper[i] = (it/divider);
cout << (it/divider) << " ";
i++;
}
cout << "\n";
//  cout << "Press ENTER to continue..." << endl;

//  getchar ();

return keeper;
}
The problem is, when I run it, windows generates an error and stops running it. Is this a problem with my code, my compiler (Dev-C++) or the OS itself? (I'm using Windows 2000) Thanks! ~Dakar [edited by - Dakar on May 7, 2004 4:11:50 PM]

##### Share on other sites
quote:
Original post by nobodynews
It doesn''t look like you are allocating any memory for keeper. Ie, you are writing on a part of memory you shouldn''t, and Windows "ain''t down wit dat" if you know what I mean.

You should probably use std::vector to store the numbers, it looks much more appropriate in this instance, unless you are trying to learn how to use pointers of course.

Well, I''m still a Newbie to programming, and this program is mainly to learn pointers.

Thanks.
~Dakar

##### Share on other sites
also you only really have to check the numbers between 1 and divident/2, (because dividing by more than divident/2 will result in a number lower than 2, which obviously wont be a natural number unless it is 1) might help efficiency if you are searching for factors of large numbers. just a thought

##### Share on other sites
I've done some improvements (such as making the for loop (it/2), and allocating memory for keeper) and now it works, under one condition: I enter a prime number. Then it writes "5570636". nothing else works. Am I doing something wrong, or should I switch compilers?

EDIT: new code:
#include <iostream>#include <cstdio>#include <cstdlib>using namespace std;int *divide(int it, int *keeper);int main()      //makes a divider program{  int divident;  int *list;  int *listo;  cout << "This program divides a number by several others and shows the results.\n What shall the number be?";  cin >> divident;  cout << "\n";  divide(divident, list);  cout << list[1];  system("PAUSE");  return 0;}  int *divide(int it, int *keeper){  keeper = new int;  int divider;  int i;  int total;  for (divider = 2; divider <= (it/2); divider++)  {  if (!(it % divider))// this was suggested by leiavoia  {   total = (it / divider);   keeper[i] = total;   cout << total << " ";   i++;  }  }  cout << "\n";//  cout << "Press ENTER to continue..." << endl;//  getchar ();  return keeper;}

Also, I've commented out a couple of lines and it runs quite well if ( keeper = total; ) and ( cout << list[1]; ), so the problem is probably there.

Thanks!
~Dakar

[edited by - Dakar on May 7, 2004 7:21:44 PM]

[edited by - Dakar on May 7, 2004 7:29:42 PM]

##### Share on other sites
If it is a prime number then it hasn't dividers right? So in this case what the function is returning is in reality the pointer you gives it (i.e. the address)...

At least that is what I think... You know I'm also pretty new with C++ (altough I've been programming in VB for a while)

Also... try declaring "i" initializing it to zero, this may be a problem too...

Another thing, 512 works fine, but if you put 1024 it crashes, don't know why, sorry...

[edited by - VBBR on May 7, 2004 7:39:55 PM]

##### Share on other sites
quote:
Original post by VBBR
If it is a prime number then it hasn''t dividers right? So in this case what the function is returning is in reality the pointer you gives it (i.e. the address)...

At least that is what I think... You know I''m also pretty new with C++ (altough I''ve been programming in VB for a while)

Well,( cout << list ) apparently displays the address of list.

I think the problem has to do with using keeper as an array. If I remove the brackets it will run.

##### Share on other sites
1) A pointer needs to point at something. If you're going to use it as an array, then you probably want to allocate an array of things that it points at instead of just a single thing. Think: How many times does your loop execute? Therefore, how many spots will you need in the array?

2) Assigning to an input parameter (in a useful way) requires pass-by-reference. Otherwise you're just reusing the name. Noone outside the function sees any change made to a pass-by-value parameter. Of course, an easier fix is to just allocate the memory outside the function - put your new[] statement in main() *before* you call the function.

OR, you could define the function so that it *returns* a new int[] containing the division results, and delete[] it in main() after you're done with the values. This way of doing things is a lot more friendly to the Java idiom though (since in Java there is no real equivalent to delete/delete[], the system is periodically scanning for unreachable bits of memory and freeing them up for you).

[edited by - Zahlman on May 7, 2004 10:25:34 PM]

##### Share on other sites
quote:
Original post by Zahlman
1) A pointer needs to point at something. If you''re going to use it as an array, then you probably want to allocate an array of things that it points at instead of just a single thing. Think: How many times does your loop execute? Therefore, how many spots will you need in the array?

The thing is, the only way to know how many spots are needed is to know all of the factors (minus itself and one) that are in the number.

quote:
2) Assigning to an input parameter (in a useful way) requires pass-by-reference. Otherwise you''re just reusing the name. Noone outside the function sees any change made to a pass-by-value parameter. Of course, an easier fix is to just allocate the memory outside the function - put your new[] statement in main() *before* you call the function.

OR, you could define the function so that it *returns* a new int[] containing the division results, and delete[] it in main() after you''re done with the values. This way of doing things is a lot more friendly to the Java idiom though (since in Java there is no real equivalent to delete/delete[], the system is periodically scanning for unreachable bits of memory and freeing them up for you).

[edited by - Zahlman on May 7, 2004 10:25:34 PM]

Sorry, but I don''t understand the last two paragaraphs. I''ll try to figure it out.

Anyway, I''ve fixed the problem. Apparently, I did not assign 0 to i, which made the compiler go on fritz. It now works.

Thanks anyway,
~Dakar.

##### Share on other sites
quote:
Original post by Dakar
quote:
Original post by Zahlman
1) A pointer needs to point at something. If you're going to use it as an array, then you probably want to allocate an array of things that it points at instead of just a single thing. Think: How many times does your loop execute? Therefore, how many spots will you need in the array?

The thing is, the only way to know how many spots are needed is to know all of the factors (minus itself and one) that are in the number.

Sure, but you can put an upper bound on it, at least.
Or if that's unacceptable, consider std::vector.

And yeah. Just as a general rule:

void foo(int input) {  ...  input = /* STOP! Chances are you are writing a bug. The function that calls foo()             doesn't see the change to the value. */

In My Ideal Programming Language(TM), it is easy to use parameters as "out" parameters, but difficult to mess up. The best idea I've come up for that is a combination of Java-like object semantics, auto-boxing (or maybe just do things the Smalltalk way? I assume it treats objects similarly to how Java does WRT pointer/reference semantics?) and an explicit prohibition against assigning to the input variables. So:
function foo(Integer param) : int  Integer tmp = something;  tmp = somethingElse; // ok  param = somethingElse; // not allowed omg!!!11  param._ = 42; // ok, assigning to the value field. I think I like C++ style "const correctness" better than the idiom of making objects immutable  foo = param._; // this is one thing that Pascal etc. did very nicely IMHO - maybe it's just my style, but I tend to end up with a lot of variables named "result" whose purpose in life is to be returned...   return;

[edited by - Oluseyi on May 9, 2004 2:28:44 AM]

##### Share on other sites
quote:
Original post by Zahlman
And yeah. Just as a general rule:

void foo(int input) {  ...  input = /* STOP! Chances are you are writing a bug. The function that calls foo()             doesn't see the change to the value. */

[edited by - Oluseyi on May 9, 2004 2:28:44 AM]

I think I see what you mean. Since it is a pass by value, main() won't see the change to input. But wouldn't it work if the return value Was input? It would also probably work if it was a pass by reference.

Just trying to clarify what I've read...

Thanks!
~Dakar

Edit: I've finished the code (using arrays instead of pointers), and another problem has cropped up: whenever I run it, it displays random numbers at the end instead of the common factors. Can anybody help?

#include <iostream>#include <cstdio>#include <cstdlib>using namespace std;int *divide(int it, int keeper[]);int real(int fac[], int fac2[]);int main()      //makes a divider program{  int divident, divident2;  cout << "This program divides a number by several others and shows the results.\n What shall the number be?";  cin >> divident;  int list[divident];  cout << "\n";  divide(divident, list);  cout << "\n\n" << "Now enter a larger number.\n";  cin >> divident2;  int listo[divident2];  cout << endl;  divide(divident2, list);  real(listo, list);  system("PAUSE");  return 0;}  int *divide(int it, int keeper[]){  int divider, total;  int i = 0;  keeper[i] = it;  i++;  for (divider = 2; divider<=(it / 2); divider++)  {   if (!(it % divider))// this was suggested by leiavoia   {    total = (it / divider);    keeper[i] = total;    cout << total << " ";    i++;   }  }  keeper[i] = 1;  cout << "\n";  return keeper;}  int real(int fac[], int fac2[])  {   int r, s;   cout << "common factors are:";   for (r = 0; fac[r]; r++)   {    for (s = 0; fac2[s]; s++)    {     if (fac2[s] == fac2[r])     {      cout << fac[r] << " ";     }    }   }   cout << "\n";   return 0;  }

[edited by - Dakar on May 9, 2004 11:22:45 AM]

##### Share on other sites
I've found the problem... I was using fac2 twice. Now it simply does not display anything.

Does anybody know what I'm doing wrong?

EDIT: Now I think I see (possibly one of) the problem(s). The loops keep on going until the end of the array, which is MUCH farther than the last occupied slot. Still, I don't now how to fix this. Help?

~Thanks
Dakar.

[edited by - Dakar on May 9, 2004 9:27:13 PM]

##### Share on other sites
The loop does not go "to the end of the array"; it goes until the first zero-valued element. If nothing in your array is zero (quite possible; the unused memory is uninitialized and could hold anything), then you have a crash (access violation). What you need to do is remember the count of how many elements are meaningful in there.

Or, you could just use the property that the common factors of two numbers are simply the factors (including itself) of the greatest common factor of the two numbers. Note that there is a well known algorithm for finding GCD''s.

Again, strongly consider std::vector. It will keep track of this resizing/knowing-current-size business for you.

Also, you passed "list" to the factor-finding function the second time as well; I assume you meant to pass "listo" so as to have two separate sets of data to compare.

Also, "real" is a strange name for that function.

##### Share on other sites
quote:
Original post by Zahlman
The loop does not go "to the end of the array"; it goes until the first zero-valued element. If nothing in your array is zero (quite possible; the unused memory is uninitialized and could hold anything), then you have a crash (access violation). What you need to do is remember the count of how many elements are meaningful in there.

Thanks, I''ve now made sure the last needed array space is null.

quote:
Or, you could just use the property that the common factors of two numbers are simply the factors (including itself) of the greatest common factor of the two numbers. Note that there is a well known algorithm for finding GCD''s.

I see. I''ll try and work that in.

quote:
Again, strongly consider std::vector. It will keep track of this resizing/knowing-current-size business for you.

Also, you passed "list" to the factor-finding function the second time as well; I assume you meant to pass "listo" so as to have two separate sets of data to compare.

Also, "real" is a strange name for that function.

Sorry, but I don''t know what a vector is in C++.

The list thing was the problem. Now it works perfectly. Thanks!

Real was short for "Realize", though I have no idea why I wanted it to be calld that. I changed its name to factor.

Thanks again!
~Dakar.