Критика моего кода

[AcTiV]

хочу посмотреть на свой код с другой стороны, критерии:
-синтаксис
-читаемость
-быстродействие
ну что там еще есть

PHP код:


#include "stdafx.h"
#include "iostream"
#include "Windows.h"
#include "Math.h"
using namespace std;


//глобальные переменные и константы
const double PI = 3.14159265;

//=====================начало функции калькулятора=======================//
void Calculator ()
{
    //переменные
    float x, y;    
    int SelectToOption;
    //основа функции калькулятор
    cout << "\nФункция калькулятор" << endl;
    cout << "Выберите действие калькулятора:" << endl;
    cout << "1 - умножение\n2 - деление\n3 - додавание\n4 - отнимание" << endl;
    cin >> SelectToOption;
    //умножение
    if ( SelectToOption == 1)
    {
        cout << "Вы выбрали действие умножение" << endl;
        cout << "Введите первое число: " ;
        cin >> x;
        cout << "Введите второе число: " << endl;
        cin >> y;
        cout << "Умножение " << x << " на " << y << " равно " << x * y << endl;
    }
    //деление
    else if (SelectToOption == 2 )
    {
        cout << "Вы выбрали действие деление" << endl;
        cout << "Введите первое число: " ;
        cin >> x;
        cout << "Введите второе число: " ;
        cin >> y;
        cout << "Деление " << x << " на " << y << " равно " << x * y << endl;
    }
    //додавание
    else if ( SelectToOption == 3 )
    {
        cout << "Вы выбрали действие додавание" << endl;
        cout << "Введите первое число: " ;
        cin >> x;
        cout << "Введите второе число: " << endl;
        cin >> y;
        cout << "додавание " << x << " и " << y << " равно " << x * y << endl;
    }
    //отнимание
    else if ( SelectToOption == 4 )
    {
        cout << "Вы выбрали действие отнимание" << endl;
        cout << "Введите первое число: " ;
        cin >> x;
        cout << "Введите второе число: " << endl;
        cin >> y;
        cout << "отнимание " << x << " и " << y << " равно " << x * y << endl;
    }
    else 
    {
        cout << "Неправильный ввод. Функции под номером " << SelectToOption << " нету\n" << endl;
        Calculator();
    }
}
//=====================================конец функции калькулятора====================//



//====================================начало математических функций================//
void MathFunction ()
{
    //переменные
    int SelectToMathF;    //выбор функции
    double l;            //переменная для мат. функций
    /*
    1 - sin        2 - cos        3 - tg        4 - ctg        5  - arcsin        6 - arccos        7 - arctg        8 - arcctg
    9 - Ln        10 - Log10    11 - Sqrt        12 - Exp
    */

    cout << "Вы выбрали раздел математичесие функции" << endl;
    cout << "Для выбора функции, напишите ее номер (  )" << endl;
    cout << "Список математических функций:\n" 
        << "1)Sin \n2)Cos \n3)Tg \n4)Ctg \n5)ArcSin \n6)ArcCos \n7)ArcTg \n8)ArcCtg \n9)Ln \n10)Log10 \n11)Sqrt \n12)Exp \n" << endl;
    cout << "Введите номер функции: " ;
    cin >> SelectToMathF;
    //=======определяет что выбрал пользователь==========//
    //функция синус
    if (SelectToMathF == 1)
    {
        cout << "Вы выбрали математическую функцию Sin" << endl;
        cout << "Напишите число для Sin(x): " ;
        cin >> l;
        cout << "Sin(x) = " << sin(l) << endl;
    }
    //функция косинус
    else if (SelectToMathF == 2)
    {
        cout << "Вы выбрали математическую функцию Cos" << endl;
        cout << "Напишите число для Cos(x): " ;
        cin >> l;
        cout << "Cos(x) = " << cos(l) << endl;
    }
    //функция тангенс
    else if (SelectToMathF == 3)
    {
        cout << "Вы выбрали математическую функцию Tg" << endl;
        cout << "Напишите число для Tg(x): " ;
        cin >> l;
        cout << "Tg(x) = " << tan(l) << endl;
    }
    //функция котангенс
    else if (SelectToMathF == 4)
    {
        cout << "Вы выбрали математическую функцию Ctg" << endl;
        cout << "Напишите число для Ctg(x): " ;
        cin >> l;
        cout << "Ctg(x) = " << 1 / tan(l) << endl;
    } 


[AcTiV]

PHP код:

    //функция арксинус
    else if (SelectToMathF == 5)
    {
        cout << "Вы выбрали математическую функцию ArcSin" << endl;
        cout << "Напишите число для ArcSin(x): " ;
        cin >> l;
        cout << "ArcSin(x) = " << asin(l) << endl;
    }
    //функция арккосинус
    else if (SelectToMathF == 6)
    {
        cout << "Вы выбрали математическую функцию ArcCos" << endl;
        cout << "Напишите число для ArcCos(x): " ;
        cin >> l;
        cout << "ArcCos(x) = " << acos(l) << endl;
    }
    //функция арктангенс
    else if (SelectToMathF == 7)
    {
        cout << "Вы выбрали математическую функцию ArcTg" << endl;
        cout << "Напишите число для ArcTg(x): " ;
        cin >> l;
        cout << "ArcTg(x) = " << atan(l) << endl;
    }
    //функция арккотангенс
    else if (SelectToMathF == 8)
    {
        cout << "Вы выбрали математическую функцию ArcCtg" << endl;
        cout << "Напишите число для ArcCtg(x): " ;
        cin >> l;
        cout << "ArcCtg(x) = " << (PI/2)-atan(l) << endl;            
    }
    //функция натуральный логарифм
    else if (SelectToMathF == 9)
    {
        cout << "Вы выбрали математическую функцию Ln" << endl;
        cout << "Напишите число для Ln(x): " ;
        cin >> l;
        if (l > 0)
        {
            cout << "Ln(x) = " << log(l) << endl;        
        }
        else
        {
            cout << "Ввденное число меньше нуля" << endl;
            MathFunction();
        }
    }
    //функция десятичный логарифм
    else if (SelectToMathF == 10)
    {
        cout << "Вы выбрали математическую функцию Log" << endl;
        cout << "Напишите число для Log(x): " ;
        cin >> l;
        if(l > 0)
            cout << "Log10 (десятичный) = " << log10(l) << endl;
        else
        {
            cout << "\n\t\t\tНЕПРАВИЛЬНЫЙ ВВОД" << endl;
            MathFunction();
        }
    }
    //функция корень
    else if (SelectToMathF == 11)
    {
        cout << "Вы выбрали математическую функцию Sqrt" << endl;
        cout << "Напишите число для Sqrt(x): " ;
        cin >> l;
        if(l > 0)
            cout << "Sqrt = " << sqrt(l) << endl;
        else
        {
            cout << "\n\t\t\tНЕПРАВИЛЬНЫЙ ВВОД" << endl;
            MathFunction();
        }
    }
    else if (SelectToMathF == 12)
    {
        cout << "Вы выбрали математическую функцию Exp" << endl;
        cout << "Напишите число для Exp(x): " ;
        cin >> l;
        if(l > 0)
            cout << "Exp = " << exp(l) << endl;
        else
        {
            cout << "\n\t\t\tНЕПРАВИЛЬНЫЙ ВВОД" << endl;
            MathFunction();
        }
    }
    else
    {
        cout << "\n\t\t\tНЕПРАВИЛЬНЫЙ ВВОД" << endl;
        MathFunction();
    }
}
//===============================конец математических функций=========================// 


[AcTiV]

PHP код:

//=============================начало геометрических формул=============================//
void FormulsOfGeometrix ()
{
    //переменные
    double l;    //кут
    double h;    //высота
    double a;    //
    double b;    //стороны
    double c;    //
    double p;    //периметр
    double r;    //радиус
    int SelectToForOfGeom;    //переменная для сравнивания с вводом пользователя
    //для паралелограмма
    int yes = 1, no = 0, YesOrNo;

    cout << "Вы выбрали раздел 3. Геометрические формулы" << endl;
    cout << "Что бы выбрать, просто введите номер формулы и нажмите Enter" << endl;
    cout << "Список формул: \n"
        << "1)Площадь квадрата\n"
        << "2)Площадь прямоугольника\n"
        << "3)Площадь паралелограмма\n"
        << "4)Площадь ромба\n"
        << "5)Площадь трапеции\n"
        << "6)Площадь круга\n"
        << "7)Площадь эллипса\n"
        << "8)Площадь Равнобедренный треугольника\n"
        << "9)Площадь Равносторонний треугольника\n" << endl;

    cin >> SelectToForOfGeom;
    //определяет что выбрал пользователь
    //формула квадрата
    if (SelectToForOfGeom == 1)
    {
        cout << "Вы выбрали площадь квадрата" << endl;
        cout << "Введите сторону квадрата: " ;
        cin >> a;
        if (a > 0)    
            cout << "Площадь квадрата = " << pow(a, 2) << endl;
        else
        {
            cout << "\nНЕПРАВИЛЬНЫЙ ВВОД. Сторона квадрата меньша или равна 0\n" << endl;
            FormulsOfGeometrix();
        }
    }
    //формула прямоугольника
    else if (SelectToForOfGeom == 2 )
    {
        cout << "Вы выбрали площадь прямоугольника" << endl;
        cout << "Введите первую сторону: " ;
        cin >> a;
        cout << "Введите вторую сторону: " ;
        cin >> b;
        if ((a && b) > 0)
            cout << "Площадь прямоугольника = " << a * b << endl;
        else
        {
            cout << "\nНЕПРАВИЛЬНЫЙ ВВОД. Сторона квадрата меньша или равна 0\n" << endl;
            FormulsOfGeometrix();
        }
    }
    //формула Параллелограмма
    else if (SelectToForOfGeom == 3)
    {
        cout << "Вам известна высота и сторона? (1 - да, 2 - нет)" << endl;
        cin >> YesOrNo;
        //известна ширина и высота
        if (YesOrNo == 1)
        {
            cout << "Введите высоту: " ;
            cin >> h;
            cout << "Введите ширину: " ;
            cin >> a;
            if ((a && h) > 0)
                cout << "Площадь паралелограмма = " << a * h << endl;
            else
            {
                cout << "НЕПРАВИЛЬНЫЙ ВВОД. Стороны должныть быть больше чем 0" << endl;
                FormulsOfGeometrix();
            }
        }
        //известно 2 стороны и кут
        else
        {
            cout << "Введите длину: " ;
            cin >> a;
            cout << "Введите ширину: " ;
            cin >> b;
            cout << "Введите кут (sin): " ;
            cin >> l;
            //проверяем что стороны не меньше 0
            if ((a && b && l) > 0 )
                cout << "Площадь паралелограмма = " << a * b * sin(l) << endl;
            else
            {
                cout << "НЕПРАВИЛЬНЫЙ ВВОД. Стороны должныть быть больше чем 0" << endl;
                FormulsOfGeometrix();
            }
        }
    }
    else
    {
        cout << "\n\t\t\tНЕПРАВИЛЬНЫЙ ВВОД" << endl;
        FormulsOfGeometrix();
    }
}
//============================конец геометрических формул=============================//



//первая функция, название программы, идея и операции которые будет исполнять программа
void Hello ()
{
    cout << "\t\t  =================================================" << endl;
    cout << "\t\t  ||   \t\t   Добрый День\t\t\t ||" << endl;
    cout << "\t\t  ||          Это мой первый большой проэкт\t ||" << endl;
    cout << "\t\t  ||    Math.Function by AcTiV"
        << "  V 1.0 (Console)\t ||" << endl;
    cout << "\t\t  =================================================\n\n" << endl;
}
//=====================================================================================// 


[Morpho]

Вместо кучи else if можно было бы использовать switch..

[ROD]

const double PI = 3.14159265;

Уберите. В Math.h уже есть PI, а так же некоторые другие константы, посмотрите сами (они там сделаны через препроцессор #define).

Ну и кроме того, не могу не согласится с комментарием выше.

Вот собственно пример того, как надо делать:

switch(SelectToOption)
{
   case 1:
      cout << "Вы выбрали действие умножение" << endl;
      ...
      break;
   case 2:
      cout << "Вы выбрали действие деление" << endl;
      ...
      break;
   default://если ни один из кэйсов не выбран
      cout << "Такого действия нет. Попробуйте еще раз." << endl;
      ...
      break;
}

Могу отметить, что вы хотя бы читабельно код оформляете. Многим [новичкам] этого не хватает.

[Bugrimov]

ROD прав. Самый оптимальный вариант, на мой взгляд, использовать switch

[ACE Valery]

Названия переменных не фонтан.
SelectToForOfGeom - это как переводить-то? Если уж называете переменными длинным именем, то не сокращайте слова. К тому же ваш инглиш мне не понятен.
И не нужно создавать переменных с именем вроде YesOrNo. Неужели нельзя создать yes, и если она = 1, то да, если 0, то нет. Вот объясните мне, что можно подумать, увидев такую переменную? В моем понимании в ней должны храниться не числа, а именно 'yes' или 'no'.

И уберите магические числа! 1,2,3,4 в условиях. Какую смысловую нагузку они несут? Я б никогда не подумала, что 11 - это корень, если б не было комментов. Но хороший код должен быть понятен без комментов. В С++ есть такая чудная штука как define. Советую воспользоваться

[AcTiV]

я учту ваши пожелания насчет улучшения кода

[_Bers]

Сообщение от ACE Valery

. В С++ есть такая чудная штука как define. Советую воспользоваться

Не советую.

[ROD]

Сообщение от _Bers

Не советую.

А, если не секрет, по чему?

#define FUNC_SUM 1
#define FUNC_SUB 2
#define FUCN_MUL 3
#define FUCN_DIV 4


switch(SelectToOption)
{
   case FUNC_SUM:
      ...
      break;
   case FUNC_SUB:
      ...
      break;
   case FUNC_MUL:
      ...
      break;
   case FUNC_DIV:
      ...
      break;
}

По моему читабельное и понятнее, чем "Магические числа" посреди кода, а самое главное - их легко и просто заменить - всегда знаешь точно где они определены.

Переменные использовать - не рационально (хотя компилятор может эту проблему решить, а может и не решить).