Looking for constructive criticism!

[jakeford]

We're working with file streaming, and reading and writing to files. I managed to complete the assignment, but I feel there was probably a better way to have done it.. Here is the assignment my professor gave us:

Prompt the user to determine if they know their weight in kilograms. If they know the weight(answering yes to the question), the program will read a file containing the weight in kilograms and convert it to pounds. If they don't know the weight(answering no to the question), the program will prompt them to enter their weight in pounds and convert it to kilograms and store it in the file.

#include <iostream>
#include <fstream>

using namespace std;

int main()
{
    ifstream inFile;
    ofstream outFile;

    double kilograms, pounds;
    char userAnswer;

    inFile.open("answeredYes.txt");
    outFile.open("answeredNo.out");

    cout << "Do you know your weight in kilograms?(y/n)" << endl;
    cin >> userAnswer;

    if(userAnswer=='y'){
            cout << "Enter your weight in kilograms:" << endl;
            cin>>kilograms;
            pounds = kilograms * 2.2;
            inFile >> kilograms;
            cout << "Your weight in pounds is:" << pounds << endl;
    }while(userAnswer=='n'){
        cout << "Enter your weight in pounds:" << endl;
        cin >> pounds;
        kilograms = pounds / 2.2;
        outFile << pounds;
        cout << "Your weight in kilograms is:" << kilograms << endl;
    }

    inFile.close();
    outFile.close();

    return 0;
}

[Dy7SLyTq]

its pretty good. i can only think of some minor "optimizations" that probably wouldnt make a difference. its just my style of coding

[vlad from moscow]

The program is written very bad. At least I do not understand such constructions linked together as

if ( /*...*/ ) {

} while ( /*...*/ ) {

}

This only confuses the reader.

Moreover the code does not corresponds exactly the assignment.

[Dy7SLyTq]

its not written bad. its in a different style. its the form (or at least a variation of it) that kerninghan and ritchie use

[jakeford]

Thanks. Yeah it's not a very detailed program so there aren't too many ways to do it I guess. I was wandering if I should have used an else statement instead of while

[vlad from moscow]

@DTSCode

its not written bad. its in a different style. its the form (or at least a variation of it) that kerninghan and ritchie use

It is very advanced to name a very bad written code as a different style of writing.:)

[Dy7SLyTq]

just because it confuses you doesnt make it bad. its not badly or poorly written if its in a different style. his style is valid

[jakeford]

@vlad from moscow - It is very advanced to name a very bad written code as a different style of writing.:)

Well I came here for constructive criticism so how would you have written this program kind sir?

[vlad from moscow]

@DTSCode

just because it confuses you doesnt make it bad. its not badly or poorly written if its in a different style. his style is valid

Remember on all your life that code that confuses readers is a bad written code. It is not an "other style". It is simply a bad code. And who does not understand this is a simply a very bad programmer.

[jakeford]

Remember on all your life that code that confuses readers is a bad written code. It is not an "other style". It is simply a bad code. And who does not understand this is a simply a very bad programmer.

The program does exactly what it's supposed to if I'm not mistaken.. I've been learning c++ for maybe a month and just started taking this class, which is why I posted in the beginner section! Either way, you fuel me to excel.

[Dy7SLyTq]

thats not true. if i show a beginner to c++ the stroustrop calculator and they are confused by it that doesnt make it badly written. it means they dont understand it properly

if ( /*...*/ ) {

} while ( /*...*/ ) {

}

is a valid style. its not poorly written. it is an other style.

[Dy7SLyTq]

oh no i understand it fine, but you might need to google some things if you think any style besides your own is poorly written. it doesnt confuse everyone, just you. i understood it just fine, because as i said its a style (or a variation of it) that is used by many people, namely brian kerningham and dennis ritchie.

[nkendra]

The program looks good.

I would just note the following:

1 - When reading in input use std::string instead of char. By using char, the program will crash if the user types more than one character for the answer. Nothing else in the code would need to be changed, by switching to string.

2 - Yes, an if else statement would have worked better than a while(). Especially because userAnswer never changes inside the loop, therefore the loop will never terminate.

3 - When opening files, check to make sure the file opened successfully. Streams are able to open a file during construction:

ifstream iFile("data_file.txt");
if( !iFile)
{
  cout << "Error: Could not open input file" << endl;
  return 0;
}

3 - close() does not need to be called on the files because the ifstream and ostream destructors automatically do that for you.

[jakeford]

@nkendra,

Thanks!

[Dy7SLyTq]

what are you using cmath for?

[jakeford]

DTSCode (1120)
what are you using cmath for?

LOL great question man that needs to be taken out.. and do I need iomanip either? I think I just copy and pasted those includes before I did anything lol

[Dy7SLyTq]

no. your not manipulating output so its not needed

[ne555]

¿what's the purpose of line 26?
Also, ¿why do you have two files?

[Cubbi]

nkendra gives good advice, but to add a bit on "Streams are able to open a file during construction", your other variables are also created long before they can be used.
For example, kilograms and pounds are constructed at line 13, but only first used at lines 24 and 25. In a trivial program such as this, it's easy to look up from line 24 to line 13 to find out what kind of variable it is, but as programs grow, it becomes harder.

The general rule is to declare variables as locally as possible, best of all only when they can be initialized meaningfully (this is item 18 in the C++ Coding Standards by Sutter/Alexandrescu)

[Dy7SLyTq]

¿what's the purpose of line 26?

ne555s comment made me look at your program again. there are some mistakes that i missed after my cursory glance your streaming wrong