HiveBrain v1.2.0
Get Started
← Back to all entries
debugcppMinor

Dual-language beginner math solver program in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
programlanguagebeginnermathdualsolver

Problem

I've written a program that solves questions of:

  • 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 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 2


What 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 0


In 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 2
if (language == 1 || 2)
if language is 1, or 2 is not 0
if (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.