Optimizing code

i dont know nothing about quadratic equation either, so i dont know you use the right function. However, i found one problem in your code: if you try to run it twice, the second time you can only set the x-varaible. The reason is clear; you dont reset cCurrentVar and iInputLoop. And i would use a switch instead of if/if else/else if. This is the new code:

#include <iostream>
using namespace std;

//Power function
int pow( int iInt, int iPower ) {
	//Variables
	int i = 1;
	//Main loop
	while( i < iPower ) {
		iInt = iInt * iInt;
		i++;
	} //end while loop
	return iInt;
} //End function

//Pause function
void Pause() {
	cin.sync();
	cin.get();
} //End function

//Clear function
void Clear() {
	system("CLS");
} //End function



//Main function
int main() {
	//Variables
	int iResult;
	int iInputLoop;
	int iQuad[4];
		iQuad[1] = 0;
		iQuad[2] = 0;
		iQuad[3] = 0;
		iQuad[4] = 0;
	char cCurrentVar = 'a';

	//Main program loop
	do {
		cout << "Enter values for the variables 'a, b, c, and x'." << endl;
		Pause();
		Clear();
		iInputLoop=0;
        //Input Loop
		do {
            iInputLoop++;
			
			switch (iInputLoop)
			{
            case 1:cCurrentVar='a';break;
            case 2:cCurrentVar='b';break;
            case 3:cCurrentVar='c';break;
            case 4:cCurrentVar='x';break;
            }
			Clear();
			cout << "a = " << iQuad[1] << ", b = " << iQuad[2] << ", c = " << iQuad[3] << ", x = " << iQuad[4] << "." << endl << endl;
			cout << "( " << cCurrentVar << " ) > ";
			cin  >> iQuad[iInputLoop];
            
		} while( iInputLoop < 4 ); //End input loop
        Clear();
		//Print function
		cout << iQuad[1] << "(" << iQuad[4] << ")^2 + " << iQuad[2] << "(" << iQuad[4] << ") + " << iQuad[3] << " = 0";
		Pause();
		Clear();
		
		//Calculate results
		iResult = pow(iQuad[1] * iQuad[4], 2) + ( iQuad[2] * iQuad[4] ) + iQuad[3];
		cout << "Press enter for your results!";
		Pause();
		Clear();
		
		//Print results
		cout << "Your result is: " << endl << endl;
		cout << iResult << "!";
		Pause();
		Clear();

		} while( true ); //End main program loop

		return 0;
} //End main

I would also build in a stop, for example:

bool repeat=true;
...
do
{
...
if (iQuad[1]==0 && iQuad[2]==0 && iQuad[3]==0 && iQuad[4]==0)
repeat=false;
}while (repeat)

Last edited on

Ok. Some things to change in your code.

Line 9: Change this to a for-loop. For-loops with a fixed upper limit can be easily optimised, a while loop not so much.

Line 35: Arrays always start at 0 and goto Limit - 1. So iQuad[4] (line 38) is invalid. It's only valid from 0-3.

Line 59: Same as above,
Line 66 Same again
Line 71 Same again.

Line 63: Make into a for-loop. Will run faster if unrolled by compiler

Last edited on

You're of course right about both things. Some stupid mistakes.

However, i dont understand one thing:
I have tested the code before posting it, and it worked fine. I agree iQuad[4] is invalid, so why didnt I got an error?

Undefined behaviour. It will tend to work in smaller applications because that memory is likely not allocated elsewhere. However, in larger applications that memory is surely going to be used. So by changing the value of it you end up changing an unknown value and having undefined behaviour.

oke, thanks. i get it now

I know that arrays start at 0, but I get a strange error when I use: iQuad[3] = {0,0,0,0}

It says it has too many initializers, but it's fine when I use iQuad[4].

Other than that, thanks for the advice, changing now. =]

That's because iQuad[3] represents an array with 3 elements. These elements are index from 0-2.

So I should still use iQuad[4] to initialize, but only use up to iQuad[3] in my code?

Yes. When declaring the number tells the compiler how many elements total. Then you can use from 0 upto that number-1.

http://www.cplusplus.com/doc/tutorial/arrays.html

Last edited on

[EDIT]

Added another code to first post, please review it. =]

Dont use a loop that calls itself. That gets really slow with big numbers. I would suggest someting like this:

#include <iostream>

//gets input of the user, without crash ;)
int getint(int min,int max)
{
  int input;
  char temp[100];
  while (input<min||input>max||(input==0 && temp[0]!='0'))
  {
  std::cin.getline(temp, 100);
  input=strtol(temp,0,10);
  if (input<min||input>max||(input==0 && temp[0]!='0'))
  std::cout<<"Not an option!\n";
  }
}

using namespace std;

int main()
{
    int oldvalue[2]={1,1};
    int newvalue;
    int input;
    std::cout<<"How many numbers?\n";
    input=getint(1,46);
    cout<<oldvalue[0]<<",\n"<<oldvalue[1]<<",\n";
    for (int i=3;i<=input;i++)
    {
        newvalue=oldvalue[0]+oldvalue[1];
        oldvalue[0]=oldvalue[1];
        oldvalue[1]=newvalue;
        cout<<newvalue;
        if (i<input)
        {
            cout<<",\n";
        }
        //here you can add the next\quit stuff
    }
    cin.ignore();
    return 0;
}

There is a problem in the code above i dont get: it works fine for the first 46 numbers, and after that it start to put out negative numbers...?

So i set the userinput to maximaal 46 :) It is a lot faster than your code.

Last edited on

Fluffy Bunny (24)

You can also do it without any loops. Don't know if it is faster.

#include <iostream>
#include <cmath>
using namespace std;

int main()
{
  int n,m;
  cin >> n;
  m = pow((1+sqrt(5))/2,n)/sqrt(5) + .5;
  cout << "The "<<n<<"th Fibonacci number is "<< m <<".\n";
}

I dont totally understand how your code works, but its problebly faster, since it only calculates once :)

But your code has the same problem as mine: after the 46st number, it start to put out negative numbers (different negative numbers then mine program, wich makes it even stranger).
Can anyone tell me wy both programs put out negative values after 46?

Fluffy Bunny (24)

The algorithm is from Wikipedia. There is an explicit formula for the nth Fibonacci number.

The program must be failing after 46 because the 47th Fibonacci number is > 2^31, so is too big to be an int.