Looking for constructive criticism!

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

I don't doubt that it could be classified as "a style" the same as writing all code on one line could be classified as "a style". The fact that it can be classified as such doesn't make it well (or even not poorly) written.

I certainly can't imagine anyone with any depth of experience looking at this and thinking it was an example of good style, and I've certainly never seen code written by an experienced person that looked that way.

One wonders why the program keeps asking if your choice is 'n' but only asks once if your choice is 'y'. Your while is formatted as if it was an 'else' to the original 'if', but it is not although it would make more sense that way both in terms of logic and consistency in program behavior.

One also wonders what you're doing with the inFile and outFile. You don't actually use anything you read in from inFile. Why does choosing yes or no affect whether you're writing to or reading from a file? Another inconsistency there.

Since I can't really tell what you're trying to do with the file i/o from your code, here's an alternate version that ignores that:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
#include <string>
#include <iostream>

double doWeightConversion(const std::string& inUnit, const std::string& outUnit, double ratio)
{
    double units ;
    std::cout << "Enter your weight in " << inUnit << ": " ;
    std::cin >> units ;

    double result = units * ratio ;

    std::cout << "Your weight in " << outUnit << " is: " << result << std::endl ;

    return result ;
}

int main()
{
    const double poundsPerKilo = 2.20462 ;
    const double kilosPerPound = 1.0 / poundsPerKilo ;

    char userAnswer;

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

    if ( userAnswer == 'y' )
        doWeightConversion("kilograms", "pounds", poundsPerKilo) ;
    else if ( userAnswer == 'n' )
        doWeightConversion("pounds", "kilograms", kilosPerPound) ;
}
Last edited on
closed account (Dy7SLyTq)
i was mainly arguing the point that just because he was confused by it that it doesnt make it a bad style and that is a validated style. however i wont argue that its not a bad style. i consider it a good style but its a matter of opinion
@cire


Thanks. As my English is not good I did not want to spend my time explaining what is bad and what is good in programming You did it instead of me.:)
@DTSCode
i consider it a good style but its a matter of opinion


There are two opinions. The first one is the opinion of those who nothing understands in programming as you but has his own opinion.:).
And the other one is of experienced programmers.
Last edited on
closed account (Dy7SLyTq)
i understand programming. its not my fault you didnt explain yourself clearly. if you look at this link: http://www.cplusplus.com/forum/beginner/106530/2/#msg576705 i corrected myself and said that the commands used need to be changed. you however only mentioned the way he wrote it and did not make it clear exactly what you are talking about. since that was all you mentioned, that was all i thought you were talking about which is perfectly valid. so you cant say i dont understand programming because you didnt explain yourself properly
@DTSCode

i understand programming. its not my fault you didnt explain yourself clearly


I am sorry however I am not your professor that to explain you all in detail.:)
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.

This is the part of the program that I had trouble with.. I feel like my professor could have worded these instructions better..
closed account (Dy7SLyTq)
if you want to argue something you need to explain it in detail. i am done talking to you because your pointless arguement just shows how much you really know.
Anyways guys thanks for all of the advice!
I'm just going to throw it out there that while I hate your brackets and inconsistent spacing between operators and operands (it's been beaten to death in this thread already) you've got some very good use of spacing between lines which I too often stop doing halfway through a program.

You may want to allow the characters 'Y' & 'N' to be entered as well as that is something a user would commonly enter into a program.
I'm just going to throw it out there that while I hate your brackets and inconsistent spacing between operators and operands (it's been beaten to death in this thread already) you've got some very good use of spacing between lines which I too often stop doing halfway through a program.


To clarify, the brackets and spacing didn't really have much to do with it. It was chaining a while with an if that is unsupportable.

1
2
3
4
5
if ( ... ) {
    ...
} while ( ... ) {
    ...
}


The two blocks of code are not necessarily logically related and this construct makes it appear that the OP thinks while and else can be freely interchanged with each other, although I suppose spacing might be considered part of the issue since inserting a newline immediately before the while would alleviate the situation (my personal preference would be for a blank line to separate the two blocks.)
Last edited on
The two blocks of code are not necessarily logically related and this construct makes it appear that the OP thinks while and else can be freely interchanged with each other, although I suppose spacing might be considered part of the issue since inserting a newline immediately before the while would alleviate the situation (my personal preference would be for a blank line to separate the two blocks.)


Yeah I'm just now getting to the 4th chapter in my class. This is where I will learn how to use loop statements. My instructor kind of jumped ahead when he assigned this. He probably didn't realize that it requires a loop to do this assignment, smh.
Btw has anyone else read any of D.S. Malik's books? I'm reading from problem analysis to program design 6th edition
> He probably didn't realize that it requires a loop to do this assignment

You do not need a loop for this assignment (in which input validation, error checking and recovery have not been demanded).

The if-while construct which has been passionately debated is not germane to this particular assignment. The only flow-control construct it requires is a simple if-else

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.


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
#include <iostream>
#include <fstream>
#include <string>

int main()
{
    const double pounds_per_kilogram = 2.3 ;
    const char* file_name = "weight_in_kg.txt" ;

    // Prompt the user to determine if they know their weight in kilograms.
    std::cout << "do you know your weight in kiligrams? (yes/no): " ;
    std::string answer ;
    std::cin >> answer ;

    //  If they know the weight(answering yes to the question)
    if( answer == "yes" )
    {
         // the program will read a file containing the weight in kilograms
         std::ifstream file( file_name ) ;
         double weight_in_kilograms ;
         file >> weight_in_kilograms ;

         // and convert it to pounds
         const double weight_in_pounds = weight_in_kilograms * pounds_per_kilogram ;
         std::cout << "kilograms: " << weight_in_kilograms 
                   << "  pounds: " << weight_in_pounds << '\n' ;
    }

    // If they don't know the weight(answering no to the question)
    else if( answer == "no" )
    {
         // the program will prompt them to enter their weight in pounds
         double weight_in_pounds ;
         std::cout << "enter your weight in pounds: " ;
         std::cin >> weight_in_pounds ;

         // and convert it to kilograms
         const double weight_in_kilograms = weight_in_pounds / pounds_per_kilogram ;
         std::cout << "pounds: " << weight_in_pounds 
                   << "  kilograms: " << weight_in_kilograms << '\n' ;

         // and store it in the file.
         std::ofstream file( file_name ) ;
         file << weight_in_kilograms << '\n' ;
    }
}
@JLBorges

Thanks man.

I see where my mistakes were with the file reading
Topic archived. No new replies allowed.
Pages: 12