Variable length array that seems to work, but doesn't?

[The Brahmnic Boy]

I want to store the factors of a user input value Q and store in a[].
I know that the size of a[] is less than Q.
It works in Sequential programming but in OOP, it gets finicky.

I am in Dev C++ in Windows 10.

I can't do this

class Number
{
	private:
	int Q,N,R,F;
		//Q is input number
		//F is number of Factors
		//R is remainder
	int a[Q];//Array to store factors	 

Says it's illegal to use Q in that way, and array length must be specified.
(I take in the value of Q through a function accessed by an object)

(array has to be used by other functions within the class)

Of course, I can do

int a[100];

But it is very inefficient as numbers like 12 have only 6 factors.

I did this:

  class Number
{
	private:
	int Q,N,R,F;
		//Q is input number
		//F is number of Factors
		//R is remainder
	int a[];//Array to store factors		 

And it seemed to work, it runs the program well for Prime numbers...other numbers, not so well.
It does accept inputs and display data, but as it reaches the end i.e. this part:

void greatestfac()
{
     if(F<=2)
     {
	cout<<"Your number is prime. It doesn't have a greatest factor less than itself.";			
     }
     else
     {
	cout<<"The greatest factor of your number is:"<<a[F-2];
     }	
}

, it displays this:

"
Program.exe has stopped working
OK CANCEL
"

When I press enter, it dutifully displays the output.
(Once again, this only happens when the number is not prime.)

Also, the formatting options are very weird here. Couldn't use Tab.

[Rakib771]

The problem here is that array cannot be defined without a constant. So, Q has to be constant before you declare the array i.e. it has to have a value. For example,

int n;
int a[n];

will give error. But,

int n;
std::cin>>n;
int a[n];

works fine.

[Handy Andy]

Hello The Brahmnic Boy,

It has been my understanding that C++ does not allow VLAs. An array like you are using must have a constant value. This could be a number between the []s or a variable defined as const or constexpr, (since C++11), Along with this you will need a variable to keep track of how much of the array is used.

I have also read that some older compilers do still allow VLAs, but I am not sure which ones they are. Your DEV C++ might be one of them.

With out more code I question this:

class Number
{
	private:
	int Q,N,R,F;
		//Q is input number
		//F is number of Factors
		//R is remainder
	int a[Q];//Array to store factors 

You define "Q", but I can not see if it receives a value before it is used, so "Q" may have a garbage value that is unacceptable for an array size.

Given the code that you have posted something like this might work:

const size_t MAXSIZE = 25;

class Number
{
	private:
	int Q,N,R,F, arrUsed{};  // <--- "arrUsed" to keep track of the part of the array that is used.
		//Q is input number
		//F is number of Factors
		//R is remainder
	int a[MAXSIZE];//Array to store factors 

An alternative to the array would be to use a "std::vector" which does not need a preset size. The vector will grow to what it needs to hold. Using the vector class member function ".size()" will tell how many elements that the vector contains. When you are finished I believe that vectorName.clear(); will empty the vector of all elements and let you start over.

Hope that helps,

Andy

[dhayden]

Rakib771 wrote:

But,

int n;
std::cin>>n;
int a[n];

works fine.

Variable length arrays actually aren't standard C++.

Brahmnic Boy You should use a vector for a, and set both Q and A in a constructor:

#include <vector>
class Number
{
	private:
	int Q,N,R,F;
		//Q is input number
		//F is number of Factors
		//R is remainder
	std::vector<int> a;//Array to store factors
public:
    Number(int q) : Q(q), a(q), R(0), F(0)
        {}
};

Actually, you will probably find that you don't need to preallocate a specific size for a. Just add new values to the vector as needed.

[Ganado]

Also, if you were really worried about efficiency, then you would measure. It might be that simply having a fixed-size array might be really fast, or maybe it's insignificant either way.

I'm not even sure what the first number that would overflow an array of size 100. I guess 2^101? Well beyond the capability of a 64-bit integer input.

Personally, I would take into account that many numbers have repeated factors. So for example, the number 50400 factors as 2^5 * 3^2 * 5^2 * 7; you could fit that representation in an array of size 8. (Space vs time trade-off)

[The Brahmnic Boy]

*facepalm* You guys seemed to have misunderstood what I said.

Handy Andy

I did say that

int a[100];

worked fine.

I am asking why

int a[];

DOES work perfectly, but only for prime numbers.
And gives a pseudo error message for other values of Q.

(Pseudo in the sense, it gives an error stating my program has stopped working, but it gives output.)
(I should be happy that it is working, but I sense that the error message might be something else I am unaware of.)

I was trying to avoid posting the entire code.

[Ganado]

You're dealing with undefined behavior, so reasoning why something crashes is a bit silly to me. Nevertheless, we wouldn't have a clue without seeing the code (or really, the assembly). It might be that the compiler is still allocating one spot for the array, so anything above that breaks.

[The Brahmnic Boy]

Okay:

#include<iostream>
#include<conio.h>
using namespace std;

class Number
{
	private:
	int Q,N,R,F;
		//Q is input number
		//F is number of Factors
		//R is remainder
	int a[];//Array to store factors	
	public:
		void getdata()
		{
			cout<<"Enter the integer you want to find the factors of: \n \t";
			cin>>Q;
		}	
		void calculate()
		{
			int m=0; //Array index counter
			F=0;
			for(N=1;N<=Q;N++)
			{
				R=Q%N;
				if(R==0) //Condition for N being a factor
				{
					a[m]=N; //Store factor in Array
					F++; //Number of Factors Increased by one
					m++; //Array index increased by one
				}
			}
		}	
		void numfactor()
		{
			cout<<"\n There are "<<F<<" factors of "<<Q<<".";
		}
		void listfactor()
		{
			cout<<"\n They are:"<<endl;	
			for(int i=0;i<=Q;i++)  //Print factors
			{
				cout<<"\t"<<a[i];
				if(a[i]==Q)
				{
					break;
				}
			}
		}
		void sumfactors()
		{
			long s=0;
			cout<<" \n The sum of those factors is: "<<endl;
			for(int p=0;p<=Q;p++)
			{
				s=s+a[p];
				if(a[p]==Q)
				{
					break;
				}
			}
			cout<<"\t"<<s<<endl;	
		}
		void greatestfac()
		{
			if(F==2)
			{
				cout<<"Your number is prime. It doesn't have a greatest factor less than itself.";			
			}
			else
			{
				cout<<"The greatest factor of your number is:"<<a[F-2];
			}	
		}
};

int main()
{
	Number num1;
	num1.getdata();
	num1.calculate();
	num1.numfactor();
	num1.listfactor();
	num1.sumfactors();
	num1.greatestfac();	
	getch();
}

[jonnin]

int a[];//Array to store factors

accessing a will crash you. It has no memory.
this is identical to saying
int *a = null;
a[19] = 32; //crash, if you are lucky. if not it will just corrupt data here and there.

you need to give it an initial size. if you hate vectors or can't use them just give it a nice fat size for now:
int a[100000];

if you can use vectors, preallocation of a bunch is still an OK idea. If it gets big you can use push_back instead of [] notation to load the array.
vector <int> a(100000);
a[100] = 11; //fine

I don't know if you have any lectures on how memory works. But basically, if you access memory incorrectly, there are 3 scenarios.
1) it crashes because the OS thinks you are doing something wrong and stops you cold.
2) if reading, it works, because the memory belongs to your program, but it isn't what you thought it was.
3) if writing and it works, you damage a different variable or even code!

none of these are very good. 1 is the best outcome, because you HAVE to fix it to get it running. 2 and 3 can look like it is working, even when it is doing something wrong, and worse, it may work all the time until you put it on another computer or run a bigger problem and then can't explain the problems that crop up.

worse again, debug compiles like to put crap around variables in memory so if you go out of bounds, you don't do 'real' damage, only the debugger itself is affected, and since it works, you don't use the debugger, then you do your release build and the problems begin.

[Handy Andy]

Hello The Brahmnic Boy,

You are correct. I misunderstood the whole of what you were saying being focused on just one small part.

Just as a[Q] works when "Q" is defined as a constant number for a[100] works because the "100" is a constant number. a[] does not work because you are trying to define a (0)zero length array. This (0) zero length array is not allowed.

Something that would work is a[]{ 1, 2, 3, 4, 5 };. This is using the initial values here to set the size of the array to (5), i.e., the elements would be 0 - 4 for a total of 5.

Whether you use a[Q] or a[] and if it should work comes down to the DEV C++ IDE setup and the compiler. DEV C++ is an old program and I do not recall what standard the compiler used before I changed it to use C++11 standard. Because you are likely using C++98 standards or something older. As Ganado mentioned the a[] may be allocating one space for the array.

When I installed DEV C++ I eventually found where to adjust the IDE to use the C++11 standards. Since I have changed it I do not recall what the original setting was. If you are interested in updating the IDE let me know. It would be a good idea to use, at a minimum, the C++11 standards.

Hope that helps,

Andy

[The Brahmnic Boy]

Jonin

My computer Science course includes Assembly language programming.

Although it is in 8085. Not sure if that helps at all.

----------------------------------------------------------

Is this one of the cases where OOP seems to not perform well?

I am sure there will be plenty of cases in which the size of the array has to be determined by another variable, and it should be simple as:

int Q;
cin>>Q;
int a[Q];

But in OOP, you have to declare all the data members and arrays before you even get input from the user.

BTW, I was happy when my instructor gave us this challenge because I did this program in sequential programming. Now I know why he just smiled and said it wasn't that easy in OOP.

[Ganado]

First, this really has nothing to do with the OOP paradigm. It's simply about how to properly define/initialize an array.

Doing:

int Q;
cin>>Q;
int a[Q];

is simply not allowed in standard C++. It's a GNU extension called "VLA" (Variable-Length Array, which actually is standard in C, but still potentially dangerous to use).
The only difference with putting it in a class is that a is initialized when the object is initialized, so it's already too late to give the array a size; thus a is always allocated with a size of 0.

About your code:
Again, it's simply undefined behavior. So what you're doing is simply not allowed, regardless of if it happens to work.

I don't know assembly very well at all, so I'm actually guessing here, but this is what the GNU assembly looks like:
using

g++ -S main.cpp -o main_asm.s

(Note: my g++ compiler is 64-bit, yours might be 32-bit, but so the registers might have different names, but the behavior should be similar)

main:
.LFB1525:
	pushq	%rbp
	.seh_pushreg	%rbp
	movq	%rsp, %rbp
	.seh_setframe	%rbp, 0
	subq	$48, %rsp
	.seh_stackalloc	48
	.seh_endprologue
	call	__main
	;
	; ... (removed for brevity)
	;
	leaq	-16(%rbp), %rax
	movq	%rax, %rcx
	call	_ZN6Number11greatestfacEv
	movl	$0, %eax
	addq	$48, %rsp
	popq	%rbp
	ret
	.seh_endproc
	.def	__tcf_0;	.scl	3;	.type	32;	.endef
	.seh_proc	__tcf_0

Note that the sizeof(Number) in your program is 16, because you have 4 ints, each int taking up 4 bytes, but a[] takes up 0 size, although the address of a goes past what was allocated for your Number object, so already something is amiss.

In your greatestfac number, it compares a value to 2, and if it isn't equal to 2, it does a jump.

	cmpl	$2, %eax
	jne	.L21

And .L21 looks like:

.L21:
	leaq	.LC9(%rip), %rdx
	movq	.refptr._ZSt4cout(%rip), %rcx
	call	_ZStlsISt11char_traitsIcEERSt13basic_ostreamIcT_ES5_PKc
	movq	%rax, %rcx
	movq	16(%rbp), %rax
	movl	12(%rax), %eax
	leal	-2(%rax), %edx
	movq	16(%rbp), %rax
	movslq	%edx, %rdx
	addq	$4, %rdx
	movl	(%rax,%rdx,4), %eax
	movl	%eax, %edx
	call	_ZNSolsEi

My guess is that something within the .LN21 logic (i.e. greater than 2 factors) is corrupting the stack, and causing the program to fail when it eventually tries to pop off the stack.

If you actually changed your class to look like, e.g.

class Number {
	int Q,N,R,F, dummy;
	int a[];
};

sizeof(Number) will now be 20.

You'll notice that the assembly now will do
subq $64, %rsp
instead of
subq $48, %rsp,
giving you 16 more bytes to work with (4 ints).

So, entering 4 will now not crash your program.
Now, even entering 16 won't crash your program.
But entering something that produces a longer result, like 32, will still crash your program.

If you wanted to explore exactly which part of the assembly is failing, you might want to look into a tool like gdb or related tools.
https://stackoverflow.com/a/55935724/8690169

[jlb]

In C++ both a[Q] (where Q is a variable) nor a[] should compile unless you are using compiler specific extensions.

But in C both a[Q] (where Q is a variable) or a[] (as the last variable in a structure) might compile when compiling to the C99 standard. VLA are legal in C99, and optionally legal in C11, the second is called a flexible array and that was added with the C99 standard.

But realize that both of these methods have limitations of when and/or how they can be used and many compilers have compiler specific hacks that may or may not meet the standard definition.

I really don't recommend either VLA or flexible array for beginner to intermediate programmers, leave them to the "experts". Also if you do plan to use them then I recommend you make sure you're using the standard versions, and avoid the compiler specific hacks.

[Ganado]

One interesting note is that Visual Studio is actually stricter by default than GCC. GCC requires the -pedantic-errors flag to be passed to not compile int a[]; but Visual Studio seems to not compile it by default.

[jlb]

but Visual Studio seems to not compile it by default.

Okay, how are you compiling? C or C++?

Remember that Visual Studio only truly supports C98, although they have been adding the parts of modern C that are required for C++.

Since C++ doesn't support either of the "features" in question and C98 doesn't support these "features" it doesn't surprise me that it is stricter.

But you're correct, GCC requires several compile flags to disable all their compiler "hacks". Just setting the the -std flag isn't enough.

[Ganado]

C++. I'm not arguing with you, everything you said was correct.

[jonnin]

int Q;
cin>>Q;
int a[Q];

I am not here to tell you "should".
the c++ compiler makes flat C arrays in the data segment (in assembly terms) with a fixed size the same way assembly does them.

the way you make a variable sized thing in assembly or C or C++ is the exact same way: you ask the OS for heap memory of that size. If you want to resize the thing, you ask for a bigger chunk, copy the original to it, and throw away the original. Try it... try writing a variable length assembly array and see what you need to do to make it happen. The reason will become clear: the compile-time reserved memory area has a FIXED SIZE. Some C++ compilers support the code you wrote, but it is a nonstandard extension that is doing something for you.

C++ makes this very easy:
int Q;
cin >> Q;
vector<int> a(Q); //done. c++ does the dirty work behind the scenes in this "object oriented array" type for you. technically Q would be unsigned and nonzero, if you are allowing the user to type it. You can also just not have this: vectors can grow to fit, so you can keep appending to them. This is inefficient (it does the new memory/ copy /thing I said above frequently) so giving them a large starting size to help offset this penalty is useful. all that to say I would probably have written it 'professionally' as
unsigned int Q;
cin >> Q;
vector<int> a(std::min(arrayminsize, Q); //handles 0 input and ensures it has some room to grow in case user was sloppy on providing the size.


But in OOP, you have to declare all the data members and arrays before you even get input from the user.
-- Not true, in OOP you will use vectors for your scenario and in fact for most places where you wanted an array. I am old and cranky and set in my ways and I still use arrays if I know how big I want it, but the generally accepted practice is to use vectors unless you have an odd requirement. There is also a std::array construct that is halfway between C arrays and vectors.


BTW, I was happy when my instructor gave us this challenge because I did this program in sequential programming. Now I know why he just smiled and said it wasn't that easy in OOP.
This isn't why. As you can see above, I did what you did with the same real effort just slightly different syntax. OOP complicates things in other ways, depending on how OSO you approach things (that's objects for the sake of having objects design). I am ok with mixing procedural and OOP code based on the requirement, but some places want full OOP everything and that gets bloated and annoying quickly when you have to write 300 accessor functions for the sake of writing 300 accessor functions. (this is where you start writing local programs that write blocks of code for you, which is a tool everyone will need if they get into an OSO coding shop). OOP is great, but like anything, too much of a good thing can be a giant mess.

---------------
Yes, visual studio is the better product in a lot of ways. I don't have it right now; I probably should get it :)

[The Brahmnic Boy]

The thing I gained from this post was that my high should really teach us what are vectors in C++ and how to properly use them.

Till then I'll just set this to int a[1000] and hope nobody types a number with 1000+ factors.

Also, I have marked this problem as solved.

[keskiverto]

You don't need to hope. You can enforce:

constexpr size_t CAP {1000};
int a[CAP];

size_t size {0};
while ( size < CAP && std::cin >> data ) { // read at most CAP values
  a[size] = data;
  ++size;
}

// use a
for ( size_t elem=0; elem < size; ++elem ) {
  std::cout << a[elem];
}

[jonnin]

Till then I'll just set this to int a[1000] and hope nobody types a number with 1000+ factors.

this technique worked fine for decades, with a trap similar to above to tell the user that they had gone beyond the scope of the program's capability. Its a bit of a dated technique because all modern languages support something akin to vector, but for the ones that do not, its still a valid approach so long as you tell the user what went wrong.