debugcppMinor
Dual-language beginner math solver program in C++
Viewed 0 times
programlanguagebeginnermathdualsolver
Problem
I've written a program that solves questions of:
I've also written it to be viewed in English or French. There's a few errors regarding
Please give your opinions and some advice.
```
#include
#include
#include
#include
using namespace std;
int language;
void output(string en, string fr) {
if (language == 1)
cout > a;
intcheck(a);
output("Enter the second number: ", "\nEntrez le second nombre : ");
cin >> b;
intcheck(b);
cout > a;
intcheck(a);
output("Enter the second length: ", "Entrez la seconde longueur : ");
cin >> b;
intcheck(b);
cout > a;
intcheck(a);
output("Enter the second length : ", "Entrez la deuxieme longueur : ");
cin >> b;
intcheck(b);
output("Enter the third length : ", "Entrez la troisieme longueur : ");
cin >> c;
intcheck(c);
if (a b && a > c) { // a is the largest side
lrgst = a;
l1 = b;
l2 = c;
}
else if (b > a && b > c) { // b is the largest side
lrgst = b;
l1 = a;
l2 = c;
}
else if (c > a && c > b) { // c is the largest side
lrgst = c;
l1 = a;
l2 = b;
}
else { // a = b = c
output("It's an equilateral triangle.\n", "\nC'est un triangle equilateral.\n");
return;
}
}
else {
output("\nIt's not a triangle!", "\nCe n'est pas un triangle !\n");
return;
}
cout > language;
intcheck(language);
if (language == 1 || 2) {
cout > input;
if (input == "1") {
addition();
}
else if (input == "2") {
hypotenuse();
}
- Addition
- Finding the hypotenuse of a triangle
- Checking if the triangle has a right-angle or not
I've also written it to be viewed in English or French. There's a few errors regarding
int and while but it still works all the same. In case of any exception, I just terminate the program altogether.Please give your opinions and some advice.
```
#include
#include
#include
#include
using namespace std;
int language;
void output(string en, string fr) {
if (language == 1)
cout > a;
intcheck(a);
output("Enter the second number: ", "\nEntrez le second nombre : ");
cin >> b;
intcheck(b);
cout > a;
intcheck(a);
output("Enter the second length: ", "Entrez la seconde longueur : ");
cin >> b;
intcheck(b);
cout > a;
intcheck(a);
output("Enter the second length : ", "Entrez la deuxieme longueur : ");
cin >> b;
intcheck(b);
output("Enter the third length : ", "Entrez la troisieme longueur : ");
cin >> c;
intcheck(c);
if (a b && a > c) { // a is the largest side
lrgst = a;
l1 = b;
l2 = c;
}
else if (b > a && b > c) { // b is the largest side
lrgst = b;
l1 = a;
l2 = c;
}
else if (c > a && c > b) { // c is the largest side
lrgst = c;
l1 = a;
l2 = b;
}
else { // a = b = c
output("It's an equilateral triangle.\n", "\nC'est un triangle equilateral.\n");
return;
}
}
else {
output("\nIt's not a triangle!", "\nCe n'est pas un triangle !\n");
return;
}
cout > language;
intcheck(language);
if (language == 1 || 2) {
cout > input;
if (input == "1") {
addition();
}
else if (input == "2") {
hypotenuse();
}
Solution
Where's the menu?
I'd consider moving the call to
Linguistics
You're current approach of having an output function that takes in both English and French works, however it isn't particularly scale-able. If you introduce a third language you need another parameter etc. A better approach is to abstract the lookup away from the code. Instead of embedding the strings directly when needed, instead a well known identifier for the text can be used. So you have something like
Variable naming
Some of your variable names could be improved. Consider:
What do the variables
Language Looping
Your first while loop is redundant, and also contains unnecessary checks. After you get input from the user you call
If == ||
This line:
Is actually equivalent to:
In other words, it always triggers. It should be:
intcheck
Your
Which you would call as:
namespace std
Whilst it can make things easier when you're getting started, lots of people don't like this:
It can cause name conflicts and unexpected errors, so is generally a bad habit to get into.
I'd consider moving the call to
menu to within your while loop. At the moment, if you manage to complete one of the calculations you're met with the prompt 'Selection:', which is fine if you remember what the menu looked like, but not particularly helpful if you don't. At a minimum, have the ERROR! condition show the menu to give the user a chance to pick a menu item...Linguistics
You're current approach of having an output function that takes in both English and French works, however it isn't particularly scale-able. If you introduce a third language you need another parameter etc. A better approach is to abstract the lookup away from the code. Instead of embedding the strings directly when needed, instead a well known identifier for the text can be used. So you have something like
output(language, get_menu_selection_text_id). This could initially be driven from lookup tables, however it would be better if the strings could be read in from a resource/text file. This also allows somebody other than the developer to perform the translations between languages. This approach also makes remaining strings in your code stand out more, should there be a translation for 'An error has occured!\nType of error:' for example.Variable naming
Some of your variable names could be improved. Consider:
double a, b, c;
double lrgst; // Largest side
double l1; // Side 1
double l2; // Side 2What do the variables
a,b,c represent? If lrgst was called largest_side the comment wouldn't be needed, etc.Language Looping
Your first while loop is redundant, and also contains unnecessary checks. After you get input from the user you call
intcheck. This throws an exception if a valid number isn't input, which escapes the while loop and bypasses the if/else statement.If == ||
This line:
if (language == 1 || 2)Is actually equivalent to:
if language is 1, or 2 is not 0In other words, it always triggers. It should be:
if (language == 1 || language == 2)intcheck
Your
intcheck method behaves differently, depending on when it's called. It checks for an error on stdin and assumes that if one happened it is because a number hasn't been entered and throws a "NUMERICAL" error, unless the language hasn't been set in which case it throws a "LANGUAGE" error instead. It seems like it should probably be responsible for actually receiving the number from stdin, since it's already tied to stdin by the error checking. It should then always throw the NUMERICAL error and leave it up to the caller to be responsible for checking if the language has been set. It's also worth pointing out that whilst the method is called intcheck it actually works with doubles not ints, which is confusing. With the modification above, you might end up with something more like this:double get_number_from_user() {
double x;
cin >> x;
if (cin.fail()) {
output("\nYou can only type numbers!\n",
"\nIl faut taper uniquement des chiffres !\n");
throw "NUMERICAL";
}
return x;
}Which you would call as:
double first_value = get_number_from_user();namespace std
Whilst it can make things easier when you're getting started, lots of people don't like this:
using namespace std;It can cause name conflicts and unexpected errors, so is generally a bad habit to get into.
Code Snippets
double a, b, c;
double lrgst; // Largest side
double l1; // Side 1
double l2; // Side 2if (language == 1 || 2)if language is 1, or 2 is not 0if (language == 1 || language == 2)double get_number_from_user() {
double x;
cin >> x;
if (cin.fail()) {
output("\nYou can only type numbers!\n",
"\nIl faut taper uniquement des chiffres !\n");
throw "NUMERICAL";
}
return x;
}Context
StackExchange Code Review Q#147685, answer score: 6
Revisions (0)
No revisions yet.