C++ - Am I creating this function correctly for this program?

[Andrew Mendonca]

CSCI-15 Assignment #1 ? Functions and files review (40 points), due September 9, 2013.

 

Write a program to read the coefficients of a series of quadratic equations from a text file and print the associated roots, or appropriate errors if there are no real roots, to another text file.  The coefficients of a quadratic are the a, b and c of an expression of the form ax² + bx + c, and the roots are the values of x that make the value of the expression 0. 

 

If a == 0, the formula describes a line, and you may ignore any roots (just say it isn?t a quadratic and has no solutions), and if (b²-4ac) < 0 it has only complex roots.

 

Write a function to read one set of values from the file, using reference parameters to get the values out of the function and the function?s return value to indicate whether or not the function was able to correctly read three values.  The data error you must deal with here is too few values on the line, e.g., the line has an a value only and no b or c.  You may assume that the last line in the file is the only one with an error (if any error exists), and your function should return an error code to make the program stop processing after this error.  This restriction allows you to use stream extraction to read the file, rather than reading lines and parsing them yourself.  If you want to try doing this, that?s O.K., but get it working the easy way first.

 

Write a second function to calculate the roots, taking the coefficients through value parameters and giving back the roots (if they exist) via reference parameters.  Use the return value to indicate success at calculating roots (0), no solution (-1) or complex roots (-2).  Do not call this function if you have a data error on input.

 

Write a third function to print a reasonably formatted table (one row per call from the main loop) of the coefficients, and either the roots or (different) messages indicating the various error conditions.  It must print appropriate error messages in the cases of a data error on input and either no solution or complex roots from the calculation function.  Your table must have a reasonable title line (or lines) with legends describing what things are below it in the columns, and this table's title line must be printed inside the print function.  The function must know if it is being called for the first time (or not) to print the title line.  You may not pass this information into the print function from main().  The print function MUST do this itself.  Always print the coefficients unless you have a read error.  You must align the values in the columns in a reasonable way (if you can't align the decimal points in the columns, that's OK).  Don't worry about page breaks and new title lines if the output runs beyond one page. 

 

Your main() function will prompt for the file names (hold the names in C-strings), open the input and output files, check for file open errors appropriately, loop over the input file reading coefficients, calculating roots, and printing results to the output file until either end-of-file or error on input, and then close all the files and exit.  You may make no assumptions about how many coefficients are in the input file (e.g., you may not hold the values in arrays and process them after reading them all).

You may not use any global variables in this program.  Your variables must be declared appropriately within the functions where needed; and passed to other functions as either reference or value parameters as appropriate.  Your functions will indicate any problem they encounter by returning a value to main(), where the error must be handled appropriately.  Your functions outside main() may do only the task assigned to them, and must do that entire task.  For example, you may not check for a == 0 within main and only call the calculation function if a is not zero.  Think carefully about what data you need inside each function, and what must be passed around between functions.

 

Each correct input line will comprise three real values of the form

[optional sign][digits][decimal point][digits], or

[optional sign][digits] if integer.

The last input line might have fewer values.  For example, your data file might look like this:

 

1 1 1

1.2 -2.3 0.4

-2 -3 -4

+0 -2 8.85

2.345              (error ? only one data point)

 

These data are available as the file quadratic1.txt.

 

Here is my solution:

 

#include<iostream>
#include<iomanip>
#include<string>
#include<fstream>
#include<cmath>
using namespace std;

int quadValues(ifstream&, double&, double&, double&);
int calcRoots(double, double, double, double&, double&);
void numTable(ofstream&, double, double, double, double&, double&);

// Read each set of values from the input file.
int quadValues(ifstream &inputFile, double &x, double &y, double &z)
{
 int value; 
 double root1, root2;
 string fileName;
 ofstream outputFile;
 
 // Get the name of the input file from the user.
 cout << "Enter the name of the file: ";
 cin >> fileName;
 
 // Open the file.
 inputFile.open(fileName.c_str());
 
 // If successfully opened, process the file data.
 if(inputFile)
 {
  // Open the output file named quadratic table.txt.
  outputFile.open("quadratic result.txt");
  
  // Indicate whether function read three values.
  while(!inputFile.eof())
  {
   while(inputFile >> x >> y >> z)
      {
          calcRoots(x, y, z, root1, root2);
    numTable(outputFile, x, y, z, root1, root2);
    if(!inputFile.eof())
    {
     return 1;
    }
   }
  } 
  // Close the file.
  inputFile.close();
  // Close the output file.
  outputFile.close();
 }
 else
 {
  // Display the error message
  cout << "There was an error opening the input file.\n";
 }
}

// Calculate the roots
int calcRoots(double a, double b, double c, double &root1, double &root2)
{
 ifstream inputFile;
 
 // If there is data error, end program.
 if(!inputFile.eof())
 {
  return 1;
 }
 // If a is 0, there is no solution.
 if(a == 0)
 {
  return -1;
 }
 // If discriminant is less than 0, there are complex roots.
 else if((b*b-4*a*c) < 0)
 {
  return -2;
 }
 // Otherwise, return the real numbers.
 else
 { 
  root1 = (-b+sqrt((b*b)-(4*a*c)))/(2*a);
  root2 = (-b+sqrt((b*b)-(4*a*c)))/(2*a);  
  return 0;
 }
}

// Print results on a formatted table
void numTable(ofstream &outputFile, double a, double b, double c, double &root1, double &root2)
{
 static int spaces = 10;
 ifstream inputFile;
 
 // Write the output to the file.
 outputFile << "a" << setw(spaces) << "b" << setw(spaces) << "c" << setw(spaces)
 << "Root 1" << setw(spaces) << "Root 2" << setw(spaces) << "Errors" << endl;
 outputFile << "---------------------------------------"
 << "--------------" << endl;
 outputFile << right << a << setw(spaces) << b << setw(spaces)
 << c << setw(spaces) << root1 << setw(spaces) << root2 << endl;
 if(!inputFile.eof())
 {
  cout << "Not enough values" << endl;
 }
 if(a == 0)
 {
  cout << "No solution" << endl;
 }
 else if((b*b-4*a*c) < 0)
 {
  cout << "Complex Roots" << endl;
 }
}

// Call every function.
int main()
{
 ifstream inputFile;
 double x, y, z;

 quadValues(inputFile, x, y, z);

 return 0;
}

 

Here is my output:

 

a         b         c    Root 1    Root 2    Errors
-----------------------------------------------------
1         1         1       NaN1.13296e-317
 

 

I want to go over this program one function at a time. For the first function, which is quadValues(), I don't think I'm suppose to be calling the other two functions. In this function, is this the correct way to read one set of values from the file and return an error code if there are too few values on a line? If not, what do I need to fix?

 

quadratic1.txt

[vcfan]

how much are you paying? no western union.

[vcfan]

ok had a look at your code. few problems

 

1. your code is pretty messy. start with the main function. create the file objects, then call a function to open the 2 files by passing references to the file objects, then return. do a loop of the following (pass references to file object values)

 

call another function to check if end of file, and if not do the reading of the values,return.

now call calcRoots with the values that you read and do the calculations,return. return the code based on what the result it.

now call the output file printing function,return.

 

remember to do error checks after you return from a function back to main to determine if and what went wrong.

 

now here are some errors in the above code.

 

1.you are calling calcRoots,but are not taking the return codes. whats the point of having the function return anything if youre not going to use them? then you are doing the same work in your numTable() function before printing the results. so use the return codes,and when printing don't redo your comparisons. 

 

2. in both your calcRoots() and numTable() functions, you are creating local ifstream objects, then trying to use oef() without opening the file or using a reference of the object.

 

3. you are exiting after reading the values from the inputfile only once after calling the calcRoots() and numTables() functions. you need to loop. if (!Inputfile.eof()) needs the ! removed. youre basically saying if there is still data to be read, then exit.

[Karl L.]

vcfan's analysis is absolutely correct. Your code is very messy and your logic is somewhat difficult to follow. On top of that after I thoroughly analyzed your code and compared it to your requirements for the assignment I noticed more than a few places where you did not follow directions.

 

Your first problem is your quadValues() function. According to your instructions it is not supposed to prompt the user for an input file; that should happen in main(). You are only supposed to be passing a reference to your opened input file stream to quadValues(). It should extract the three coefficients from the line and assign them to the floating point references you passed in, nothing else. Also, although I wouldn't comment every line like you did, I remember doing that as a novice programmer as well. I do, however, recommend that you get into the habit of commenting your variable declarations and function headers. With that in mind a fixed version of quadValues() might look something like the following:

/*
Read a single set of values from the input file.

Remarks:
    Since the input file is supposed to contain the coefficients of a
    quadratic (a * x^2 + b * x + c), it only accepts a very rigid formatting. Each
    line is expected to contain exactly three numbers. Any more or less will
    trigger an error, as documented below.

Arguments:
    inputFile [in]  File stream to an ASCII text file with quadratics
    a [out]         First component of the quadratic
    b [out]         Second component of the quadratic
    c [out]         Third component of the quadratic

Return Value:
    false   ERROR! The line could not be read or was formatted improperly.
            The values of a, b, and c must not be relied upon!
    true    inputFile has been advanced and a, b, and c contain the
            correspondingly named coefficients of the quadratic equation.
*/
bool getCoefficients( ifstream &inputFile, double &a, double &b, double &c )
{
    string line; // Line read from the input file
    enum
    {
        PROC_NONE = 0,
        PROC_C = 1,
        PROC_B = 2,
        PROC_A = 3,
        PROC_END = 4,
        PROC_UNKNOWN = 5
    } processing; // Input we are processing
    
    if( !inputFile.good() ) return false;
    
    // Read a single line from the file.
    if( !getline( inputFile, line ).good() ) return false;
    
    // Make sure the line has at least the minimum number of characters
    // necessary to represent valid quadratic coefficients.
    if( line.length() < 5 ) return false;
    
    // Work around a quirk of our processing algorithm. Everything is activated
    // based on spaces, so we need at least one at the beginning and one at the
    // end. You could obsolete this by designing a better processing algorithm.
    if( !isspace( line[0] ) ) line = ' ' + line;
    if( !isspace( line[line.length() - 1] ) ) line += ' ';
    
    // Parse the string backwards to get our coefficients.
    processing = PROC_NONE;
    for( string::reverse_iterator it = line.rbegin(); it != line.rend(); it++ )
    {
        if( isspace( *it ) )
        {
            // Convert everything since the last set of spaces to a floating
            // point number, ignoring the current space of course.
            switch( processing )
            {
                case PROC_C:
                     c = atof( line.substr( line.rfind( *it ) + 1 ).c_str() );
                    break;
                case PROC_B:
                    b = atof( line.substr( line.rfind( *it ) + 1 ).c_str() );
                    break;
                case PROC_A:
                    a = atof( line.substr( line.rfind( *it ) + 1 ).c_str() );
                    break;
                case PROC_UNKNOWN:
                    return false;
            }
            
            // Replace all the spaces with the terminating character. This
            // effectively removes the spaces and anything after them by
            // truncating the string.
            do
            {
                line.erase( line.rfind( *it ) );
            } while( isspace( *++it ) );
            if( it == line.rend() ) break;
            
            // Increment our counter so we start processing the next
            // coefficient.
            processing++;
        }
        
        if( !(isdigit( *it ) || *it == '+' || *it == '-' || *it == '.') )
        {
            // The line obviously contains something it should not. Only
            // integers, +, -, and . symbols are allowed!
            return false;
        }
    }
    
    return true;
}

Similarly you shouldn't be opening the input file in either your calcRoots() or numTable() functions. In fact, your usage of them in those functions does nothing. You don't open a file before checking for end-of-file, and even if you had it wouldn't do any good. When you open a file using ifstream::open() it starts at the beginning; it does not pick up in the last place you used it in your program like you seem to suppose. calcRoots() also doesn't function as intended for another reason: you calculate the negative part of the quadratic formula twice rather than the negative once and positive once. This, however, is just a simple typo and nothing to do with your programming technique. Therefore a fixed version of calcRoots() might look like the following:

/*
Calculate the roots of the quadratic equation as indicated by its roots.

Arguments:
    a [in]      First component of the quadratic
    b [in]      Second component of the quadratic
    c [in]      Third component of the quadratic
    root1 [out] '-' root of the quadratic
    root2 [out] '+' root of the quadratic

Return Value:
    -2      The roots are complex. We cannot handle this situation.
    -1      There are no roots. This equation is unsolvable.
    0       The roots were successfully calculated.
*/
int calcRoots( double a, double b, double c, double &root1, double &root2 )
{
    // If a is 0, there is no solution.
    if( a == 0 ) return -1;
    
    // If discriminant is less than 0, there are complex roots.
    if( (b * b - 4 * a * c) < 0 ) return -2;
    
    // Otherwise, return the real numbers.
    root1 = (-b + sqrt( (b * b) - (4 * a * c)))/(2 * a);
    root2 = (-b + sqrt( (b * b) + (4 * a * c)))/(2 * a);
    return 0;
}

Unfortunately numTable() doesn't come close to meeting the instructions set forth in your instructions either. It is supposed to print a header the first time it is run. In contrast, your function prints it every time. numTable() also isn't printing all errors to the output file like it is supposed to; instead it is printing errors to stdout. Beyond that it is not taking the return value of calcRoots() into account. Instead you opted to use the same formulas as calcRoots() to determine which error message you should print. However your program might stop executing before that point because you are potentially trying to print junk values of a, b, c, root1, and root2 before checking for error conditions. A drastically improved version of this function taking into account your requirements and my criticism might look like the following:

/*
Print the results of a calculation to the output file.

Remarks:
    The first time this function is called a header will be printed describing
    the contents of the file. All regular processing will also take place.

Arguments:
    outputFile [in] [out]   File stream to write our results to
    calc [in]               Were the roots successfully calculated?
                            This parameter corresponds to the return value of
                            calcRoots().
    a [in]                  First component of the quadratic
    b [in]                  Second component of the quadratic
    c [in]                  Third component of the quadratic
    root1 [out]             First root of the quadratic
    root2 [out]             Second root of the quadratic
*/
void printQuadratic( ofstream &outputFile, int calc, double a = 0, double b = 0, double c = 0, double root1 = 0, double root2 = 0 )
{
    const unsigned int line_length = 80; // Length of a line in the output file
    const int precision = 5; // Precision of the floating point numbers we are printing
    static unsigned int invocations = 0; // Number of times this function has been called
    streampos ins_pos; // Insertion position
    
    // Print a header to our output file if this is our first invocation.
    if( ++invocations == 1 )
    {
        const char * main_title = "Roots of Quadratics (a * x^2 + b * x + c)"; // Title of the output file
        
        // Print the header.
        outputFile << setfill( '-' ) << setw( line_length ) << '-' << setfill( ' ' ) << endl;
        outputFile << setw( line_length / 2 - strlen( main_title ) / 2 ) << ' ' << main_title << endl;
        outputFile << setw( line_length / 5 ) << left << "[a]";
        outputFile << setw( line_length / 5 ) << left << "[b]";
        outputFile << setw( line_length / 5 ) << left << "[c]";
        outputFile << setw( line_length / 5 ) << left << "[first root]";
        outputFile << "[second root]" << endl;
        outputFile << setfill( '-' ) << setw( line_length ) << '-' << setfill( ' ' ) << endl;
    };
    
    switch( calc )
    {
        case 0:
            outputFile << setprecision( precision ) << setw( line_length / 5 ) << left << a;
            outputFile << setprecision( precision ) << setw( line_length / 5 ) << left << b;
            outputFile << setprecision( precision ) << setw( line_length / 5 ) << left << c;
            outputFile << setprecision( precision ) << setw( line_length / 5 ) << left << root1;
            outputFile << setprecision( precision ) << root2 << endl;
            break;
        case -1:
            outputFile << "No solution!" << endl;
            break;
        case -2:
            outputFile << "Complex roots! Calculation not attempted." << endl;
            break;
        default:
            outputFile << "Read error! The line was formatted improperly." << endl;
            break;
    }
    
    // Print a footer in case this is our last call, then reset our position in
    // the output file so we are ready to accept more calculations.
    ins_pos = outputFile.tellp();
    outputFile << setfill( '-' ) << setw( line_length ) << '-' << setfill( ' ' ) << endl << endl;
    outputFile.seekp( ins_pos );
}

Finally you are lacking code for most of the processing that should be taking place in main(). You have some of it incorrectly implemented in quadValues(), but even that is far from sufficient. You should prompt for both input and output file names in main() before attempting to open them, report errors, and eventually pass them by reference to your other functions. Although you put it in the wrong place, your code prompting for an input file name is quadValues() is correct. After opening the files you need a loop in main() that calls quadValues(), calcRoots(), and numTable() according to the limitations set forth in your instructions. Overall this should be fairly simple. You have most of the code for this in place already (just in the wrong function). Following your instructions and my previous examples your main() might look something like the following:

/*
Process an input file with the coefficients of quadratic equations.
*/
int main( int argc, char * argv[] )
{
    string inputFileName; // Name and path of the input file
    ifstream inputFile; // File stream to an ASCII text file with quadratics
    
    string outputFileName; // Name and path of the output file
    ofstream outputFile; // File stream to write our results to
    
    double a; // First component of the quadratic
    double b; // Second component of the quadratic
    double c; // Third component of the quadratic
    
    double root1; // First root of the quadratic
    double root2; // Second root of the quadratic
    
    // Get the name of the input file from the user.
    cout << "Enter the name of the input file: ";
    cin >> inputFileName;
    
    // Open the input file.
    inputFile.open( inputFileName.c_str() );
    if( !inputFile )
    {
        cerr << "File could not be opened for reading: " << inputFileName << endl;
        return 1;
    }
    
    // Get the name of the output file from the user.
    cout << "Enter the name of the output file: ";
    cin >> outputFileName;
    
    // Open the output file.
    outputFile.open( outputFileName.c_str() );
    if( !outputFile )
    {
        cerr << "File could not be opened for writing: " << outputFileName << endl;
        return 1;
    }
    
    do
    {
        if( getCoefficients( inputFile, a, b, c ) )
        {
            int calc; // Result of the root calculation
            
            calc = calcRoots( a, b, c, root1, root2 );
            printQuadratic( outputFile, calc, a, b, c, root1, root2 );
        }
        else
        {
            printQuadratic( outputFile, -3 );
        }
    } while( !inputFile.eof() );
    
    inputFile.close();
    outputFile.close();
    
    return 0;
}

I hope this is enough to get you started. You have a little time before the assignment is due - more than enough to effect the changes vcfan and I described. Although you should certainly carefully consider my code snipets, don't take them as canonical. You should write your own version of each function, and compile them into a coherent program. Let us know if you need any more help.

[seta-san]

why is everyone asking us to do their homework for them now? This guy is better than the last guy who couldn't figure out if-else/case statements but dang, it's not that hard. Definite not half as hard as what you'll see out on the job. Use your textbook and make sure your assignment meets the criteria of the assignment. That's all you have to do.

[The Evil Overlord]

why is everyone asking us to do their homework for them now? This guy is better than the last guy who couldn't figure out if-else/case statements but dang, it's not that hard. Definite not half as hard as what you'll see out on the job. Use your textbook and make sure your assignment meets the criteria of the assignment. That's all you have to do.

Huh? I mean I'll be the first to say, I don't know what all that code means, nor do I know what it'll do, but I didn't see the part where this is somebody's homework...

 

edit

 

wait, nevermind, I see it now

[FloatingFatMan]

why is everyone asking us to do their homework for them now? This guy is better than the last guy who couldn't figure out if-else/case statements but dang, it's not that hard. Definite not half as hard as what you'll see out on the job. Use your textbook and make sure your assignment meets the criteria of the assignment. That's all you have to do.

 

Well, at least it's a step up from VB.Net... :p

[FloatingFatMan]

I hope this is enough to get you started. You have a little time before the assignment is due - more than enough to effect the changes vcfan and I described. Although you should certainly carefully consider my code snipets, don't take them as canonical. You should write your own version of each function, and compile them into a coherent program. Let us know if you need any more help.

 

 

Dude, don't do people's homework for them, especially one post wonders... They'll never go away if we get into that habit...

[Andre S. Veteran]

xorangekiller has already given you good explanations, so I'll address some stylistic issues.
 
1) In C++, favor declaring variables as close as possible to the point where they are used, preferrably on initialization. Declaring all your variables at the top of the function was a limitation of C and is not good style in C++, or any other non-C programming language. The problem is that it creates a lot of uninitialized state and thus increases the potential for programmer errors and undefined behavior.

// bad
void myFunc() {
    int a, b, c, d;
    // lots of code which could inadvertently use a, b, c or d
    a = getA();
    b = getB();
    doSomethingWith(a, b, c, d);
} 

// good
void my Func() {     
    // lots of code which can't use a, b, c or d since they're out of scope 
    int a = getA();
    int b = getB();
    int c, d;
    doSomethingWith(a, b, c, d); 
}

// bad
int i, j, k
for (i = 0; i < 10; ++i) {
    for (j = 0; j < 10; ++j) {
         for (k = 0; k < 10; ++k) {
              doSomethingWith(myMatrix[i][j][k]);
         }
    }
} 

// good
for (int i = 0; i < 10; ++i) {
    for (int j = 0; j < 10; ++j) {
         for (int k = 0; k < 10; ++k) {
              doSomethingWith(myMatrix[i][j][k]);
         }
    }
}

2) cin >> myString; is not a reliable way of getting user input. Notably this will not do what you want if there are any blank spaces in the input. Use the global function std::getline(cin, myString) instead.

 

3) This isn't that big of a deal, but you shouldn't omit argument names in function declarations like you do at the top of your listing. In this particular case the definitions are right there so it's not too bad, but in general declarations and definitions sit in different files and a signature with no argument names is very cryptic.

[Karl L.]

Asik, your post was informative and very well said, as usual, but I actually disagree with the first of your stylistic points. I know this is mostly preference bred by habit, but I wanted to state my arguments for declaring variables at the top of the block in which they are used rather than immediately before they are needed.
 

1) In C++, favor declaring variables as close as possible to the point where they are used, preferrably on initialization. Declaring all your variables at the top of the function was a limitation of C and is not good style in C++, or any other non-C programming language. The problem is that it creates a lot of uninitialized state and thus increases the potential for programmer errors and undefined behavior.

Although it was standardized before I was born, I learned C89 so I could deal with the limitations of old versions of GCC that didn't support more modern features of C (like declaring variables anywhere in a function) by default. Although this is no longer a limitation of C or any modern, standards-compliant C compiler - and has never been a limitation of C++ or other modern languages like Java, C#, Python, and Perl - I think it is still useful in some respects. I don't advocate declaring all variables at the top of each function, but I think it is helpful to declare all variable at the top of the block in which they are used, which is at the top of a function in some cases. This practice makes it easy to spot which variables are available within the local block scope. That, combined with the descriptory comment I ascribe to each variable I instantiate, makes it much easier to debug programs in practice. This is a practice I carry into all languages I use (except for languages like Python where the lack of explicit variable instantiation drives me nuts).

 

With that in mind your second example is absolute correct. Declaring the integer inside of the for loop that uses it conforms with my practice of declaring variables within the block in which they are used and absolutely makes the code easier to read. Since I learned C++ and C99 first, both of which support this feature, it has always been one of the things that always drove me nuts about C89. Your first example is where I disagree. While it is certainly possible to misuse the variables you declared at the top of the function before you intended to, that is a bug in your function or logic rather than your style. If the parts of the function are so decoupled as to not need those variables until the function is halfway executed, it might be more prudent to split the function into a couple of functions, each with a more specific purpose. There are some potential disadvantages to this approach, especially if the function is only called once, but automatic compiler optimizations or explicit inlining can fairly easily take care of that. In my opinion the maintainability advantages far outstrip any potential negative side effects or reverse advantages.

 

Once again, Asik, I very much respect your opinion. I realize that neither you nor I are the only ones to hold our respective positions on this issue. As far as I know they are both fairly common practices. Since this thread was started by someone presumably less informed than ourselves I just wanted to state my opposing opinion on this stylistic point for the record.

[123456789A]

Can you type out your code in BASIC and I might be able to help you out.

[Andre S. Veteran]

Asik, your post was informative and very well said, as usual, but I actually disagree with the first of your stylistic points. I know this is mostly preference bred by habit, but I wanted to state my arguments for declaring variables at the top of the block in which they are used rather than immediately before they are needed.

No problem, I always enjoy a discussion about programming language issues. :) I thought my point on variable definitions was commonly accepted in practice in C++, so I checked if by any chance it was in Effective C++ by Scott Meyers, and it is!  This is item 26 in the book: "Postpone variable definitions as long as possible". Scott Meyers lists more reasons for this practice in addition to what I mentioned:

 

Not only should you postpone a variable's definition until right before you have to use the variable, you should also try to postpone the definition until you have initialization arguments for it. By doing so, you avoid constructing and destructing unneeded objects, and you avoid unnecessary default constructions. Further, you help document the purpose of variables by initializing them in contexts where their meaning is clear.

 

The point on constructors applies to many standard library types such as strings and streams, so as a rule of thumb it's more efficient to postpone variable declarations, even though some types have no constructors (i.e. plain old data types).

 

I think your argument that putting them at the top of the block makes it clear which variables are available in the block is kinda question-begging; variables are not scoped to a block unless as a rule you always declare them at the top. I see variable scope as per the language spec, i.e. from declaration to end of block, and as a very general rule I always use the smallest scope possible because that minimizes complexity. In that sense I favor postponing variable declaration for the same reason I favor private over public class members, or internal over public classes. I've also seen too many bugs caused by use of uninitialized variables, which are incredibly frustrating to track because they're random and only happen in release builds, so anything I can do to reduce unitialized state such as postponing variable declaration is an automatic win in my book.

 

Also note that in functional languages all variables are by default immutable, so it's highly unorthodox or sometimes even impossible to define a variable without assigning it its definite, useful value.

 

So while I respect your opinion I do think it is dubious and not really commonly accepted practice.

[Karl L.]

The point on constructors applies to many standard library types such as strings and streams, so as a rule of thumb it's more efficient to postpone variable declarations, even though some types have no constructors (i.e. plain old data types).

While I agree with most of your points, this one I do not. While it is technically more efficient to assign user-defined (non-fundamental) variables as needed, the effect is negated by compiler optimizations in almost every case. Even dynamic languages like Perl are able to quickly and efficiently optimize out any potential performance hit while the source is being compiled. Creating a variable and assigning it a value (in the simplest case) requires more than one CPU instruction anyway, even on modern CISC processors. Register assignments are decided by the compiler, which is able to effectively optimize based on context. In any case, I believe the the enhanced readability of the code introduced by forward declaration of variables far outweighs the potential negatives of poor optimization and buggy logic. I worry much more about optimizing my code by hand when I code for embedded devices where my program drives the hardware directly, because embedded compilers tend to be poorly optimized for compensating for the negative affects I just described. That is an adaptation programmers will learn in computer engineering courses and through hard practice, not something most programmers or computer scientists should consider.

[Andre S. Veteran]

While I agree with most of your points, this one I do not. While it is technically more efficient to assign user-defined (non-fundamental) variables as needed, the effect is negated by compiler optimizations in almost every case. Even dynamic languages like Perl are able to quickly and efficiently optimize out any potential performance hit while the source is being compiled. Creating a variable and assigning it a value (in the simplest case) requires more than one CPU instruction anyway, even on modern CISC processors. Register assignments are decided by the compiler, which is able to effectively optimize based on context.

Well, in the general case, constructors and destructors are just methods, so by declaring a variable at this point in a program you're saying you want the constructor to be called at that point and the destructor to be called at the end of the block; that's what your program means. It's possible the optimizer can move things around to try to avoid having it called uselessly, if it can determine with certainty that both the constructor and destructor are side-effect free, but in the general case that's an impossible problem to solve especially in the pointer-heavy world of C++. So I wouldn't presume too much of compiler optimizations there. In the simple following case, for instance, there's no compiler optimization possible: 

 

class MyClass {
public:
    MyClass() { cout << "Constructor\n"; }
    ~MyClass() { cout << "Destructor\n"; }
};

void Func() {
    MyClass obj;
    FunctionThatCouldThrowAnException();
}

The following only instantiates the object if an exception isn't thrown:

void Func() {
    FunctionThatCouldThrowAnException();
    MyClass obj;
}

I would like to agree with you that this is "premature optimization" but it's really not; constructors often allocate resources (RAII) so they're not trivial code that has no performance impact or that we can trust the optimizer to remove in most cases. I also think that it's fairly subjective whether seeing local variables listed at the top of a method increases readability; personally I find it makes the program harder to understand because the purpose of these variables isn't as clear as if they were declared as close as possible to their initialization and first usage. You say you remedy that with comments but perhaps you wouldn't need as many comments if you followed common practice on this.

[Karl L.]

Well, in the general case, constructors and destructors are just methods, so by declaring a variable at this point in a program you're saying you want the constructor to be called at that point and the destructor to be called at the end of the block; that's what your program means. It's possible the optimizer can move things around to try to avoid having it called uselessly, if it can determine with certainty that both the constructor and destructor are side-effect free, but in the general case that's an impossible problem to solve especially in the pointer-heavy world of C++. So I wouldn't presume too much of compiler optimizations there. In the simple following case, for instance, there's no compiler optimization possible:

You are ignoring a key component inherent in von-neuman architecture with your presumption of optimization. The limitation of variable declaration in C89 was not arbitrary. All variables had to be declared in a single block at the top of a function because that was the most efficient way to allocate memory, and compilers at the time were not sophisticated enough to reasonably handle anything else. It also helps to prevent memory fragmentation and optimize register usage. Therefore although your point about the complexity of classes is absolutely correct, it is far from the only factor affecting optimization of memory allocation. Declaring variables at the top of the block in which they are used gives the compiler more information for optimization, not less.

 

I would like to agree with you that this is "premature optimization" but it's really not; constructors often allocate resources (RAII) so they're not trivial code that has no performance impact or that we can trust the optimizer to remove in most cases. I also think that it's fairly subjective whether seeing local variables listed at the top of a method increases readability; personally I find it makes the program harder to understand because the purpose of these variables isn't as clear as if they were declared as close as possible to their initialization and first usage. You say you remedy that with comments but perhaps you wouldn't need as many comments if you followed common practice on this.

I comment each variable almost religiously. Although variables should absolutely be well named and their intent should be obvious from their usage, the fact remains that it is not always so. To make matters worse, what was crystal clear to the original author may not be clear to another programmer working on the same code. Comments are not a work-around for making variable usage clear, but a way to clearly specify my original intent so it can be easily traced and the variable is never unintentionally misused. It is my belief that if a function needs many different variables that are disparate in usage (i.e. they cannot be logically grouped together and are thus extremely spacially separated within the function) the method should be broken down into smaller parts with clearer intention.

I think we might have to agree to disagree on this point.

[Andre S. Veteran]

You are ignoring a key component inherent in von-neuman architecture with your presumption of optimization. The limitation of variable declaration in C89 was not arbitrary. All variables had to be declared in a single block at the top of a function because that was the most efficient way to allocate memory, and compilers at the time were not sophisticated enough to reasonably handle anything else. It also helps to prevent memory fragmentation and optimize register usage. Therefore although your point about the complexity of classes is absolutely correct, it is far from the only factor affecting optimization of memory allocation. Declaring variables at the top of the block in which they are used gives the compiler more information for optimization, not less.

I don't get your point. I can see how requiring variables to be declared at the top of a function would simplify the compiler implementation, allowing it to simply bump the stack pointer by the total space of these variables at the start and set it to its original value at the end of the function. This sort of engineering trade-off would have made sense for early versions of C. This naive everything-on-the-stack scheme is highly non-optimal however, and I don't see what additional information is gained for optimization there. Optimal registry usage is derived from liveness analysis for which forward declarations are just pure noise. As I see it, forward declarations likely were an engineering convenience but certainly aren't required or even helpful to generate optimal code. This also seems to be the consensus on Stackoverflow. Could you illustrate your point?

 

I agree with your point on comments and small functions, I was just pointing out that forward declarations run contrary to making code self-documenting since they make it less clear what is the actual useful scope of a variable and what its purpose is. The issue is certainly minimized by keeping functions short and with good usage of comments, but it's still not ideal.

[Lant]

There are also cases in C++ where forward declaration is simply not possible. For example assignment to references and const (or even constexpr) variables.

It would seem to me that the language simply wasn't designed for forward declaration of variables.

[Kalint]

why is everyone asking us to do their homework for them now? This guy is better than the last guy who couldn't figure out if-else/case statements but dang, it's not that hard. Definite not half as hard as what you'll see out on the job. Use your textbook and make sure your assignment meets the criteria of the assignment. That's all you have to do.

This happens every semester.  Besides it gives this sub some action

[DrJohnSmitherson]

I don't see the big deal in posting hw here. This person obviously did the project but just needs help. He didn't ask anyone to do it for him or anything of the sort. People need help. If it wasn't hw no one care. It's really no different.

[FloatingFatMan]

I don't see the big deal in posting hw here. This person obviously did the project but just needs help. He didn't ask anyone to do it for him or anything of the sort. People need help. If it wasn't hw no one care. It's really no different.

 

I don't mind it with established members, but one post wonders? That's just taking the pee.

 

If they want help from the community, they should at least be PART of the community.  Odd's are this guy just got address to Neowin from one of the VB beggers and won't bother coming back except for more help.