#cpp #инспекция_кода
Помогите, как можно улучшить программу по нахождению корней квадратного уравнения. #include#include using namespace std; double dis(double a, double b, double c);//Псевдоним функции по нахождению дискриминанта int main()// main функция { double a, b, c, xa, xb, disb;//переменные, необходимые для программы cout << "Введите коэфиециент а: ";// Следующие строчки это сбор данных cin >> a; cout << a << endl; cout << "Введите коэфиециент b: "; cin >> b; cout << b << endl; cout << "Введите свободное слагаемое с: "; cin >> c; cout << c << endl; cout << "КОНЕЧНЫЙ ВИД: " << a << "x^2" << "+(" << b << ")" << "x" << "+(" << c << ")" << "=0" << endl;//выводит конечный вид уравнения double ld;//переменная, в которую будет записан возвращаемый результат функции dis() ld = dis(a, b, c);// запись if (ld < 0)//ветвление для защиты от отрицательного дискриминаната { cout << "Ой, все, решений нет." << endl; } else { disb = sqrt(ld); //вычисляем квадратный корень xa = ((-b) + disb) / (2 * a);// находим корни xb = ((-b) - disb) / (2 * a); cout << "ПЕРВЫЙ Х РАВЕН: " << xa << endl;//выводим корни cout << "ВТОРОЙ Х РАВЕН: " << xb << endl; } } double dis(double a, double b, double c)// функця по нахождению дискриминанта { double xld; xld = ((b*b) - (4 * a*c)); cout << "Дискриминант равен: " << xld << endl; return xld; }
Ответы
Ответ 1
Программу можно "улучшить" так: решать не так в лоб, как когда-то в школьной тетрадке писали, а с учетом особенностей поведения компьютерной плавающей арифметики. В частности, существенно лучшим в этом отношении является следующий подход к решению double d = b * b - 4 * a * c; // Дискриминант double q = b >= 0 ? (-b - sqrt(d)) / 2 : (-b + sqrt(d)) / 2; // Здесь узнается наше родное `-b +- sqrt(d) / 2 * a`, // но пока что без `a` в знаменателе double x1 = q / a; double x2 = c / q; Те, кто знаком с формулами Виета, легко увидят и математическую правильность решения. А вот почему следует поступать именно так, можно почитать в классической статье (а на русском языке - у Моулера, Малькольма и того же Форсайта в книге "Машинные методы математических вычислений") Вкратце, идея заключается в том, что в плавающей арифметике во избежание потери точности рекомендуется избегать сложения относительно больших чисел, близких по абсолютному значению, но имеющих разные знаки - результат может получиться катастрофически неточным. В "лобовом" решении подвыражение -b - sqrt(d) может страдать от этой проблемы, если b отрицательно. Вышеприведенный подход при вычислении промежуточной величины q всегда выполняет сложение чисел с одинаковыми знаками.Ответ 2
#include#include Рекомендуется сортировать заголовочные файлы по алфавиту (и группировать свои и системные), меньше шансов что один будет неявно зависеть от другого, всегда понятно в какое место вставить новый. using namespace std; Никогда не пишите using namespace std;, особенно в глобальном пространстве имен. Это приводит к конфликтам имен, причем иногда ошибки компиляции не будет, но программа будет вызывать какую-то стандартную функцию вместо Вашей. double dis(double a, double b, double c);//Псевдоним функции по нахождению дискриминанта Это называется "декларация" или объявление функции, а не псевдоним. Нет никакого смысла ее тут использовать. Пишите свои функции в начале, в main() в конце программы. Не надо экономить на буквах. Переименуйте в discriminant. int main()// main функция No shit bro. Не надо писать бессмысленные коментарии, и так понятно что она main и что она функция. { double a, b, c, xa, xb, disb;//переменные, необходимые для программы Не надо объявлять несколько несвязанных переменных в одной декларации (ES10). Объявляйте переменные только в месте их использования (ES21) и всегда их инициализируйте (ES20) (если есть чем). cout << "Введите коэфиециент а: ";// Следующие строчки это сбор данных Не надо писать комментарии, пишите код. Вынесите это в функцию void collect_input(double& a, double& b, double& c) или лучше Coefficients read_coefficients(std::istream& stream), тогда название функции будет говорить о том что тут делается. cin >> a; cout << a << endl; Здесь не нужен endl, используйте '\n'. cout << "Введите коэфиециент b: "; cin >> b; cout << b << endl; cout << "Введите свободное слагаемое с: "; cin >> c; cout << c << endl; cout << "КОНЕЧНЫЙ ВИД: " << a << "x^2" << "+(" << b << ")" << "x" << "+(" << c << ")" << "=0" << endl;//выводит конечный вид уравнения Не надо писать бессмысленные комментарии, в сообщении уже написано что это. Сложный код лучше выносить в отдельные функции, например print_formula(a, b, c); double ld;//переменная, в которую будет записан возвращаемый результат функции dis() ld = dis(a, b, c);// запись Должно быть double dis = discriminant(a, b, c);. Почему вообще ld? Не используйте непонятные сокращения. Также лучше использовать вывод типов: auto dis = ...; if (ld < 0)//ветвление для защиты от отрицательного дискриминаната { cout << "Ой, все, решений нет." << endl; Не надо мешать в одну кучу вычисление корней уранения, обработку случая отсутствия корней и логирование ошибок. Вынесите вычисление корней в отдельную функцию, например bool try_solve(const Coefficients& a, Roots& x). Альтернативный вариант - std::vector solve(const std::vector & coeffs). } else { disb = sqrt(ld); //вычисляем квадратный корень xa = ((-b) + disb) / (2 * a);// находим корни xb = ((-b) - disb) / (2 * a); Должно быть auto xa = ...; или double xa = ...; cout << "ПЕРВЫЙ Х РАВЕН: " << xa << endl;//выводим корни cout << "ВТОРОЙ Х РАВЕН: " << xb << endl; } } double dis(double a, double b, double c)// функця по нахождению дискриминанта { double xld; xld = ((b*b) - (4 * a*c)); cout << "Дискриминант равен: " << xld << endl; return xld; } Не надо засовывать логи в вычисления. Если так хочется залогировать дискриминант, напишите класс или структуру, которая будет хранить промежуточные результаты вычислений, и потом их логируйте. Например: struct QuadraticEquation { QuadraticEquation(double a, double b, double c) : a(a), b(b), c(c) {} bool solve() { discr = calc_discriminant(); if (discr < 0) return false; x1 = ... return true; } double a, b, c; double discr; double x0, x1; }; В крайнем случае можно вычислить дискриминант второй раз, если так хочется его залогировать. Ответ 3
На мой взгляд, имеет смысл: Вынести функцию решения квадратного уравнения: int solve(double a, double b, double c, double *x1, double *x2) a, b, с - коэффициенты возвращаемое значение - количество корней (или -1 если их бесконечно много) *x1, *x2 - сами корни (если корней меньше двух, лишние переменные не трогаем) Сравнивать дробные числа с некоторой точностью EPS, например, 1e-7. В том числе, сравнить дискриминант с нулём как fabs(d) < EPS. Убрать функцию вычисления дискриминанта. Ввод и вывод оставить в main'е.Ответ 4
Код с минимальными правками (подправил алгоритм): #includeusing namespace std; const double EPS = 1E-5; static bool isEqual(const double& left, const double& right, const double& eps = EPS) { return abs(left - right) < EPS; } double dis(double a, double b, double c);//Псевдоним функции по нахождению дискриминанта int main()// main функция { double a, b, c, xa, xb, disb;//переменные, необходимые для программы cout << "Введите коэфиециент а: ";// Следующие строчки это сбор данных cin >> a; cout << a << endl; cout << "Введите коэфиециент b: "; cin >> b; cout << b << endl; cout << "Введите свободное слагаемое с: "; cin >> c; cout << c << endl; cout << "КОНЕЧНЫЙ ВИД: " << a << "x^2" << "+(" << b << ")" << "x" << "+(" << c << ")" << "=0" << endl;//выводит конечный вид уравнения double ld = dis(a, b, c);//переменная, в которую будет записан возвращаемый результат функции dis() if (ld < 0.0)//ветвление для защиты от отрицательного дискриминаната { cout << "Ой, все, решений нет." << endl; } else if (isEqual(0.0, a)) { if (isEqual(0.0, b)) { if (isEqual(0.0, c)) cout << "Существует бесконечное множество решений." << endl; else cout << "Ой, все, решений нет." << endl; } else cout << "Х РАВЕН: " << -c/b << endl; } else { disb = sqrt(ld); //вычисляем квадратный корень if (isEqual(0.0, b)) { cout << "Х РАВЕН: " << disb / (2*a) << endl; } else { xa = (-b + disb) / (2*a);// находим корни xb = (-b - disb) / (2*a); cout << "ПЕРВЫЙ Х РАВЕН: " << xa << endl;//выводим корни cout << "ВТОРОЙ Х РАВЕН: " << xb << endl; } } } double dis(double a, double b, double c)// функця по нахождению дискриминанта { double xld; xld = b*b - 4*a*c; cout << "Дискриминант равен: " << xld << endl; return xld; } Если вам не принципиально выдавать доп. информацию для пользователя в функции dis, то сразу напишите там return b*b - 4*a*c; во благо срабатывания RVO, либо вообще откажитесь от данной функции (ведь вы её используете всего 1 раз), ну, либо можете извратиться и запехнуть это в макрос, например #define dis(a, b, c) b*b - 4*a*c. Ну и всю логику решения (именно решения задачи, а не ввода данных), по-моему, неплохо бы вынести в отдельную функцию хотя бы. P.S. По заявкам желающих сделал все как вы любите (аля C-style, кроме потоков ввода/вывода, конечно), зато сколько внимания уделили такому, на первый взгляд, неприметному вопросу в сообществе (автору, наверное, приятно). И, да, хорошо потроллили, молодцы :)
Комментариев нет:
Отправить комментарий