LINUX.ORG.RU
 

[C++],[быдлокод] Помаленьку учу плюсы (часть 2 из много)


0

1

Я периодически буду тут на форуме задавать вопросы или обращаться с просьбой к уважаемому комьюнити.
Собственно, в очередной раз обращаюсь с просьбой:
Раскритиковать, сказать в чём улучшить, что не следует делать и что следует делать.
Постарался разобраться с классами, хотя многое пока всё равно не до конца понятно. Если есть улучшения по сравнению с предыдущим топиком - скажите, буду рад это слышать.

main.cpp
basic_invest.h

Сразу вопрос, который и так возник: стоит ли разные классы выделять в разные заголовочные файлы, естественно в случае более крупного проекта. Правильно понимаю, что их стоит разбивать про принципу функционала и того, в каких .cpp файлах они используются/не используются?

В общем - ругайте. Это полезно)


 ← 1 2 →
[#] Ответ на: комментарий от CyberTribe 31.07.2010 5:10:14  

Ты невнимательно смотрел. Смотри внимательно ещё раз - всё там элементарно получается

***## ()
[#] Ответ на: комментарий от CyberTribe 31.07.2010 5:10:14  

>Вот последний вариант мне нравится.

Как это может нравиться? Зачем ДВАЖДЫ проверять одно и то же условие? Зачем в цикл пихать действия, которые выполняются ОДИН раз - уже, по-факту, вне цикла?

***## ()
[#] Ответ на: комментарий от Led 31.07.2010 5:16:15  

Какое действие выполняется по факту один раз вне цикла?
Добавил комментарии - где я не прав? 

  do //пока значение переменной length <= 0 выполняется цикл
  { 
   cout << "Input the length of project" << endl; 
   cin >> length; // считываем значение переменной length
   if ( length <= 0 ) {  // если length <= 0 то говорим об ошибке и выполняем цикл снова - снова считываем значение
     cerr << "Wrong length of project" << endl;  
     cerr << "The length of project must be greater then 0" << endl;  
   }  
  } 
  while (length <= 0);

** ()
[#] Ответ на: комментарий от CyberTribe 31.07.2010 5:10:14  

>нормально не получится, поскольку сообщение об ошибке ввода необходимо вывести только после первой ошибки.

Виноват, "проморгал":)

Тогда так:

for (;;) {
cout << "Input the length of project" << endl;
cin >> length;
if ( length <= 0 ) {
cerr << "Wrong length of project" << endl;
cerr << "The length of project must be greater then 0"<<endl;
continue;
}
}

***## ()
[#] Ответ на: комментарий от CyberTribe 31.07.2010 5:21:58  

>Какое действие выполняется по факту один раз вне цикла?

Написал уже выше: я ошибся, "проморгав", что условие выхода по "не-ошибка", а не по "ошибка"

***## ()
[#] Ответ на: комментарий от Led 31.07.2010 5:26:25  

Да я понял. я вопрос свой раньше написал чем вы ответили)

** ()
[#] Ответ на: комментарий от anon_666 31.07.2010 5:29:22  

>Зачем бесконечный цикл?

Это я уже просто сонный, поэтому break пропустил:)

***## ()
[#] Ответ на: комментарий от Led 31.07.2010 5:35:14  

Вот исправленный код (таки заставили меня асилить LORCODE:)

for (;;) {
    cout << "Input the length of project" << endl; 
    cin >> length;
    if ( length > 0 )
        break;
    cerr << "Wrong length of project" << endl;
    cerr << "The length of project must be greater then 0" << endl;
}
***## ()
[#] Ответ на: комментарий от Led 31.07.2010 5:42:11  

Спасибо, хотя уж до этого я и сам догадался :)

** ()
[#] Ответ на: комментарий от CyberTribe 31.07.2010 5:44:35  

>хотя уж до этого я и сам догадался

Это радует.

***## ()
[#]  

Вот мои пожелания:

1. В *.h файлах следует оставлять _только_ определения класса. Всю реализацию необходимо выносить в *.cpp файлы.

2. Если уж ты взялся писать на английском, то тогда coefficients или factors, но не koefficients.

3. Пожелание освоить argc и argv. Ну или *.data файл, через который скармливать весь набор параметров. Сейчас интерактивные приложения для shell никому не интересны.

4. Я бы еще весь код, который делает реальную работу из main вынес в отдельную функцию.

** ()
[#] Ответ на: комментарий от CyberTribe 31.07.2010 2:34:27  

По main.cpp:

1. Функцию data_input следует заменить на две функции чтения. Модифицировать переданные в функцию переменные - это плохая привычка. Лучше ничего не передавать в функцию, а просто возвращать прочитанное значение. Как-то так: 

int read_length () {
    int length;
    while (true) {
        std::cout << "Input the length of the project" << std::endl;
        // TODO А по-хорошему, надо считывать строку, а потом проверять, что в этой строке число.
        // В текущей реализации будет наблюдаться очень весёлый эффект, если вместо числа ввести буковки ;)
        std::cin >> length;
        if (length > 0)
            break;
        std::cerr << "bla-bla" << std::endl;
    }
    return length;
}

long double read_discount () {
    // Полностью аналогично. Кстати, что такое discont? Я не знаю такого слова :)
    return discount;
}
Соответственно, в main() будет достаточно написать: 
int length = read_length ();              // бывший period
long double discount = read_discount ();  // бывший percent
Если захочешь поместить этот код в класс, суть не изменится: модификация аргумента, переданного по ссылке - это Зло, от которого следует избавляться.

2. Класс results прямо так и просит, чтобы ему передавали параметры не в каждый метод по отдельности, а сразу в конструктор. И оператор << ему тоже не помешает. Это типичный образец паттерна рефакторинга функциональная зависимость (feature envy).

Изменения: 

#include <ostream>
class results {
public:
    results (int length, long double discount, cach_flow const& inv, cach_flow const& rev, cf_calc const& flows, koefficients const& disc, koefficients const& infl);

    long double profit_index ();
    long double npv ();
    float profit_period ();
    // and so on...
    friend std::ostream& operator<< (std::ostream& stream, results& res);

private:
    int _length;
    long double _discount;
    // and so on...
};
В новой версии методы класса обращаются к собственным атрибутам экземпляра класса results. Например: 
long double results::npv () {
    return _flows .accum_output (_length);
}
Оператор вывода будет выглядеть следующим образом: 
std::ostream& operator<< (std::ostream& stream, results& res) {
    stream << "NPV of this project = " << res.npv () << std::endl
           << "Index of profit     = " << res.profit_index () << std::endl
           ...
           << "IRR                 = " << res.irr_calc () << std::endl;

    return stream;
}
А код в main сократится до следующего: 
results calculations (length, discount, invest, revenue, other_flows, k_disc, k_infl);
std::cout << calculations;
Конечно, в конструктор передаётся до хрена параметров, что не является признаком хорошего кода, но это можно решить дальнейшим рефакторингом используемых в results классов (который я здесь уже не буду рассматривать, это проделывается аналогично написанному выше).

3. Общие замечания.

3.1. Использовать правильное написание терминов. Мой основной язык - русский, но всё равно слова типа discont или cash режут глаз, и на их понимание требуется лишнее время.

3.2. Не использовать разные термины для одного и того же понятия. А то в одном месте length, а в другом period, в одном discont, а в другом percent. Надо определиться и использовать один и тот же термин.

3.3. Крайне желательно помечать все методы, которые не меняют содержимого класса, как const: 

long double koefficients::output (int step) const {
    return priv_disc [step];
}
Это очень полезная привычка при работе с С++. В данном примере она позволит лучше отделить инициализацию данных от использования. А по жизни она помогает писать более надёжный и безопасный код.

* ()
[#]  
Reset

Убери using из .h'ника и замени long double на double.

***** ()
[#]  
Reset

еще у тебя в h'нике куча отдельно стоящих функций без inline, либо внеси их прям внутрь классов либо поставь inline иначе с линковкой у тебя проблемы возникнут

***** ()
[#] Ответ на: комментарий от Led 31.07.2010 1:21:10  
>>-----Цитата---->>

проллетариату стОит

<<-----Цитата----<<

А ты, я смотрю, C++ раньше русского языка выучил?
Может, ну их, эти форумы? Уткнись в IDE и пиши красивый код на плюсах %)

>>-----Цитата---->>

чисткой сортиров

<<-----Цитата----<<

Е*аный стыд. Чисткой сараев.

* ()
[#] Ответ на: комментарий от Nakgidveef 31.07.2010 2:35:22  
shty
>>-----Цитата---->>

>std::cout << "Hello, World" << std::endl;

Это слишком загромождает код. std еще ладно, но например для бустовых библиотек префиксы получаются длинноваты.

<<-----Цитата----<<

use typedef, Luke :)

*** ()
[#] Ответ на: комментарий от power 31.07.2010 14:45:48  

>Е*аный стыд. Чисткой сараев.

Да, стыдно. По этой ошибке я себе слив засчитал:)

***## ()
[#]  
ShTH

> koefficients k_disc(period); k_disc.discont(percent);// creating koefficients of discount

Ужасно. Пиши по одной инструкции на строку, и почитай про стили комментариев. Всех серунов не слушай, со временем всё получится, если есть желание.

* ()
 ← 1 2 →
Development