Critique away

whew I spent a while on this though it might not look like much it has been through a long debugging process the only problem with it now is that it uses conio for getch() (which I don't know how to fix without curses(which I con't get running))


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
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
#include <cstdlib>
#include <iostream>
#include <conio.h>

using namespace std;

int wait(){
  int c;
  printf( "\n\nPress a key to continue..." );
  c = getch();
  if (c == 0 || c == 224) getch();     
}


int convert(string isFC_or_CF){
    
    long F, C;
    
    if (isFC_or_CF == "FC"){
       // Fahrenheit to Celsius
       //The formula being (F - 32 * 5 / 9) 
       cout << "Enter a value for Fahrenheit: ";
       cin >> F;
       
           if (F < 0 || F > 0){//Make sure is a number
              C = ((F - 32) * 5) / 9;
              cout << F << " degrees Fahrenheit is equivelent to " << C << " degrees Celsius Centigrade" ;
              wait();
                 }else{
              cout << "Please enter a valid number.";
              wait();
           }
           
    }else if (isFC_or_CF == "CF"){
       // Celsius to Fahrenheit 
       // the opposite of F to C conversion
       cout << "Enter a value for Celsius: ";
       cin >> C;
       
          if (C < 0 || C > 0){  //Make sure is a number
             F = ((C * 9) / 5) + 32;
             cout << C << " degrees Celsius Centigrade is equivelent to " << F << " degrees Fahrenheit " ;
             wait();
          }else{
             cout << "Please enter a valid number.";
             wait();
          } 
           
    }
}

int main()
{
    string choice;
    cout << "Type 1 to convert Fahrenheit to Celsius\n";
    cout << "Type 2 to convert Celsius to Fahrenheit\n";
    cin >> choice;
    if (choice != "1" && choice != "2"){
       cout << "Please enter a valid number.\n";
       wait();
    }else if (choice == "1"){
       //Farenheit to Celsius
       convert("FC");
       
    }else if (choice == "2"){
       //Celsius to Fahrenheit
       convert("CF");
       
    }      

          
    cin.get();
    return 0;
}
Line 9: Why printf()?
Line 25: a) zero is a number, and a valid Fahrenheit and Celsius temperature; b) for any value of x, (x<0 || x>0)==(x!=0)
Line 42: There's no such thing as "degrees Celsius centigrade". There's "degrees Celsius" and "degrees centigrade", which are synonyms. x° C is also a possibility.
Line 58: See my comment at the bottom: http://www.cplusplus.com/forum/beginner/14067/
Lines 58-69: You don't give the user a second chance to correct his input.
Line 72: I thought you were using wait() for that.

EDIT: Oh, one more thing, convert() is not returning anything, and worse still, it's directly performing I/O.
Last edited on
@Line 9:Why indeed? ... heheheeee ... right I'll just change that
@Line 25:Hmm ... how'd I miss that
@Line 42:Well .... it sounded cool ....
@Line 58:Good point
@Line 58-69:Chance thus given and loop added plus a end button
@Line 72:I actually am going to delete that line
convert() now returns 0
Whats wrong withdirect I/O?

Thanks Helios :)

V1.2 XD

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
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
#include <cstdlib>
#include <iostream>
#include <conio.h>

using namespace std;

int wait(){
  int c;
  cout << "\n\nPress a key to continue...\n";
  c = getch();
  if (c == 0 || c == 224) getch();     
}


int convert(string isFC_or_CF){
    
    long F, C;
    
    if (isFC_or_CF == "FC"){
       // Fahrenheit to Celsius
       //The formula being (F - 32 * 5 / 9) 
       cout << "Enter a value for Fahrenheit: ";
       cin >> F;
       
           if (F < 1 || F > -1){//Make sure is a number
              C = ((F - 32) * 5) / 9;
              cout << F << " degrees Fahrenheit is equivelent to " << C << " degrees Celsius." ;
              wait();
                 }else{
              cout << "Please enter a valid number.";
              wait();
           }
           
    }else if (isFC_or_CF == "CF"){
       // Celsius to Fahrenheit 
       // the opposite of F to C conversion
       cout << "Enter a value for Celsius: ";
       cin >> C;
       
          if (C < 1 || C > -1){  //Make sure is a number
             F = ((C * 9) / 5) + 32;
             cout << C << " degrees Celsius is equivelent to " << F << " degrees Fahrenheit." ;
             wait();
          }else{
             cout << "Please enter a valid number.";
             wait();
          }     
    }
    
    return 0;
}

int main()
{
while (true){
    string choice;
    cout << "Type 1 to convert Fahrenheit to Celsius.\n";
    cout << "Type 2 to convert Celsius to Fahrenheit.\n";
    cout << "Type 3 to exit.\n";
    cin >> choice;
    if (choice == "1"){
       //Farenheit to Celsius
       convert("FC");
       
    }else if (choice == "2"){
       //Celsius to Fahrenheit
       convert("CF");
       
    }else if (choice == "3"){
       //exit program
       break;
    }else if (choice != "1" && choice != "2" && choice != "3"){
       cout << "Please enter a valid number.\n";
       wait();
    }
}
          
    
    return 0;
}
Line 72: Look more carefully at my comment on that other thread. Tip: You're doing three more comparisons than necessary.
Well convert isn't converting...
TakeInputAndConvertItAndThenOutputTheResults() would be a more apt name.
But I think his point is that it shouldn't take input and perform output itself. Instead it should take parameters from main (which should take the input for it) and return a value.
woops

also I must hurry

and this must be the longest variable name I've ever seen:
TakeInputAndConvertItAndThenOutputTheResults()

Still don't see whats wrong and I'd have to run through all meh code and change it anyway :P
to put it back in main

sorry I sound hurried but I am see you guys tomorrow

Cheers :)

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
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
#include <cstdlib>
#include <iostream>
#include <conio.h>

using namespace std;

int wait(){
  int c;
  cout << "\n\nPress a key to continue...\n";
  c = getch();
  if (c == 0 || c == 224) getch();     
}


int convert(string isFC_or_CF){
    
    long F, C;
    
    if (isFC_or_CF == "FC"){
       // Fahrenheit to Celsius
       //The formula being (F - 32 * 5 / 9) 
       cout << "Enter a value for Fahrenheit: ";
       cin >> F;
       
           if (F < 1 || F > -1){//Make sure is a number
              C = ((F - 32) * 5) / 9;
              cout << F << " degrees Fahrenheit is equivelent to " << C << " degrees Celsius." ;
              wait();
                 }else{
              cout << "Please enter a valid number.";
              wait();
           }
           
    }else if (isFC_or_CF == "CF"){
       // Celsius to Fahrenheit 
       // the opposite of F to C conversion
       cout << "Enter a value for Celsius: ";
       cin >> C;
       
          if (C < 1 || C > -1){  //Make sure is a number
             F = ((C * 9) / 5) + 32;
             cout << C << " degrees Celsius is equivelent to " << F << " degrees Fahrenheit." ;
             wait();
          }else{
             cout << "Please enter a valid number.";
             wait();
          }     
    }
    
    return 0;
}

int main()
{
while (true){
    string choice;
    cout << "Type 1 to convert Fahrenheit to Celsius.\n";
    cout << "Type 2 to convert Celsius to Fahrenheit.\n";
    cout << "Type 3 to exit.\n";
    cin >> choice;
    if (choice == "1"){
       //Farenheit to Celsius
       convert("FC");
       
    }else if (choice == "2"){
       //Celsius to Fahrenheit
       convert("CF");
       
    }else if (choice == "3"){
       //exit program
       break;
    }else{
       cout << "Please enter a valid number.\n";
       wait();
    }
}
          
    
    return 0;
}
Topic archived. No new replies allowed.