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

>while (error == true);

Это скомпилируется в большую по размеру ассемблерную инструкцию, чем while (error), а читабельности не прибавляет IMHO.

> стоит ли разные классы выделять в разные заголовочные файлы

На вкус и цвет товарищей нет!) Если Висуал студио так делает - это не говорит о том, что это прально))) Хлтя конечно, надо смотреть по обстановке и полагаться на свой вкус.

Ну про вижуалстудию я не в курсе.
Сейчас пользуюсь kdevelop, исключительно как текстовым редактором. Подсветка синтаксис и контекстная «помощь» по элементам кода помогает.

>стоит ли разные классы выделять в разные заголовочные файлы

стоит

Два момента.

1. using - зло.
2. Не стоит мешать табы и пробелы для отступов, выглядит жутко. Пользуйтесь табами.

Ну, это конечно не правило, но можно делть так:
У тебя есть классы Class1, Class2, Class3. Для каждого есть файл Class<N>.h, где содержатся определения и Class<N>.cpp, где содержится сам код класса.

там везде используются табы, просто длина табуляции стоит 2 символа.

По первому - в предыдущей теме было противоположное мнение.
Предлагается использовать полные названия таких функций? Или namespac-ы?

реализация методов в basic_invest.h - зачем? Смотреть противно на такую простыню

Вот эта мысль мне, пожалуй, нравится.

>быдлокод

Ты себе льстишь. Это не быдлокод, это «ужас летящий на крыльях ночи».

Может, ну его это программирование? Можеть проллетариату стОит «своими прямыми обязаностями - чисткой сортиров» (с) Ф.Ф.Преображенский ?

Я уже понял, что их отдельно надо)
Поправлю к следующему разу.

	data_input(period, percent);// reading the length of project and percent rate
	koefficients k_disc(period); k_disc.discont(percent);// creating koefficients of discount
	koefficients k_infl(period); k_infl.inflate(percent);// creating koefficients of inflation
	cout << "Input the investment cash flow per year" << endl
	<< "Remember, that investment on the 0th age can't be zero, as it doesn't make sense." << endl;
	cash_flow invest(period); // creating investment cash flow
	cout << "Input the revenue cash flow per year" << endl;
	cash_flow revenue(period);// creating revenue cash flow
	invest.d_cash_flow(k_disc); revenue.d_cash_flow(k_disc); // discounting cash flows
	cf_calc other_flows; other_flows.calc(period, invest, revenue); // creating and calculating other flows.
	results calculations; // initialazing the results.

	// The actuall calculations and output.
	cout << "NPV of this project = " << calculations.npv (period, other_flows) << endl
	<< "Index of profit of this project = " << calculations.profit_index (period, invest, revenue) << endl
	<< "Period for getting profit = " << calculations.profit_period (period, other_flows) << endl
	<< "Equivalent annuitet = " << calculations.ea_calc (period, percent, other_flows, k_disc) << endl
	<< "MIRR of this project = " << calculations.mirr_calc (period, invest, revenue, k_disc, k_infl) << endl
	<< "IRR of this project = "<< calculations.irr_calc (period, percent, invest, revenue) << endl;

Хорошая плотность символов на см2. Пускай враги глаза сломают.

А если хочется?
Хороший стиль ведь с опытом приходит, разве нет?

Разделить строки? Между каждой - пустую? или не писать в одну строку по две операции. Пожалуй второе можно. И, может, в выводе увеличить число строк - будет аккуратней, хотя тут не уверен.

>Хороший стиль ведь с опытом приходит, разве нет?

Здесь не только в стиле дело. Здесь код даже не ремесленника, а дворника/уборщицы.

комментарии к каждой строчке. Комментарии к конструкторам лишние.

// this function is used for

эту воду тоже убрать. Комментарий должен быть кратким, по делу и только там, где он действительно нужен. Ты же код пишешь, а не сочинение. А хороший код в комментариях практически не нуждается

>Здесь не только в стиле дело. Здесь код даже не ремесленника, а дворника/уборщицы.

Да ладно тебе. Для новичка код выглядит вполне неплохо.

for ( int i = 0; i < priv_disc.size(); ++i ) {
i == 0 ? priv_disc[i] = 1 : {...}

прям конкурс по обфускации кода

Скачай какой-нибудь проект, посмотри на структуру сорцев, всяко лучше лоровских аналитиков

Ах да, код не читал

все, дальше не осилил. Размоченный свежим пивом мозг отказывается воспринимать портянку с такой плотностью символов, не стал даже пытаться понять общий смысл кода.

Что-то в таком духе лучше?

// reading the length of project and percent rate
	data_input(period, percent);
	
	// creating koefficients of discount
	koefficients k_disc(period);
	k_disc.discont(percent);
	
	// creating koefficients of inflation
	koefficients k_infl(period);
	k_infl.inflate(percent);
	
	// creating investment cash flow
	cout << "Input the investment cash flow per year" << endl
	<< "Remember, that investment on the 0th age can't be zero, as it doesn't make sense." << endl;
	cash_flow invest(period);
	
	// creating revenue cash flow
	cout << "Input the revenue cash flow per year" << endl;
	cash_flow revenue(period);
	
	// discounting cash flows
	invest.d_cash_flow(k_disc);
	revenue.d_cash_flow(k_disc);
	
	// creating and calculating other flows.
	cf_calc other_flows;
	other_flows.calc(period, invest, revenue);
	
	// initialazing the results.
	results calculations; 

	// The actuall calculations and output.
	cout << "NPV of this project = "
	<< calculations.npv (period, other_flows) << endl
	
	<< "Index of profit of this project = " 
	<< calculations.profit_index (period, invest, revenue) << endl
	
	<< "Period for getting profit = "
	<< calculations.profit_period (period, other_flows) << endl
	
	<< "Equivalent annuitet = "
	<< calculations.ea_calc (period, percent, other_flows, k_disc) << endl
	
	<< "MIRR of this project = "
	<< calculations.mirr_calc (period, invest, revenue, k_disc, k_infl) << endl
	
	<< "IRR of this project = "
	<< calculations.irr_calc (period, percent, invest, revenue) << endl;

По поводу комментариев к функциям в определении класса - они в kdevelop удобно показываются при упоминание данного класса, что может пригодиться. Касательно остальных комментариев и плотности кода - пожалуй стоит поправить. Займусь

>Для новичка код выглядит вполне неплохо.

Что именно неплохо? Нахрена вообще там эти while'ы с error'ами, когда там for(;;) c break'ом - БОЛЕЕ чем достаточно?

Что-то в таком духе лучше?

Всяко лучше. По поводу using vs explicit qualification, советую не экономить на буквах и приписывать в клиентском коде (а ваш код по отношению к stdlib именно клиентский) пространства имён явно:

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

Пишите каждый оператор на своей строке. И попробуйте camelStyle вместо snake_style, для C++ он подходит гораздо лучше. Посмотрите для примера API таких великанов как Qt, Cocoa.

> Зачем вообще там эти while'ы с error'ами, когда там for(;;) c break'ом - БОЛЕЕ чем достаточно?

do ... while интуитивно говорит, что тело цикла выполнится хотя бы один раз, а в этой программе оно именно так и должно быть. Программа пишется для человека, а не для машины. Чего там компилятору «БОЛЕЕ чем достаточно» - мало кого интересует.

Т.е. проверять условие для выхода из цикла два раза вместо одного - это нормально? Хотя, кого я спрашиваю:)

> Т.е. проверять условие для выхода из цикла два раза вместо одного - это нормально?

Там и невооружённым глазом видно, что автору есть над чем работать. Думаю со временем стиль сложится, а пока глобальной катастрофы не вижу. Мне по работе приходится иметь дело и с гораздо ужаснее кодом от «самоучек со стажем». А тут автор сам просит критики и реагирует на неё адекватно.

Имхо, заменить error на isErrorOccured, а потом уже while(!isErrorOccured). Ну, это так, на будущее, чтобы код легче читать было. В данном случае переменная error вообще бесполезная(если честно, то она почти всегда такая из-за своего абстрактного имени). Просто while(length <= 0). Для следующего цикла аналогично.

Переменные передаются по ссылке и изменяются. Это, конечно, дело вкуса, но, ИМХО, лучше out-параметры по указателю, а по ссылке in и обязательно const.

Тернарный оператор я бы тут на if поменял, но уж если очень хочется, то:

i == 0 ? priv_disc[i] = 1 : priv_disc[i] = priv_disc[i-1] / ( 1 + ( percent ) );
//может лучше так?
priv_disc[i] = (i == 0) ? 
  1 :
  priv_disc[i-1] / ( 1 + ( percent ) );

Для for и if разный стиль для скобочек - выберите что-нибудь одно. Блок объявления переменных в начале функции можно пустой строкой от кода отделить - читать будет приятнее. Слишком много комментариев. etc...

Очень благодарен за критику.

Кое что поменял. В будущем постараюсь следовать советам.

basic_invest.h http://codepad.org/YTaWTnUA
basic_invest.cpp http://codepad.org/SbCoZuOG
main.cpp http://codepad.org/A4AWWpiy

>std::cout << «Hello, World» << std::endl;

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

По тренарному и if - уже заменил. Пожалуй, if выглядит более читаемо.
Переменные передаваемые по ссылке по тому и передаю по ссылке - чтобы их можно было изменить. Где изменение не нужно - передаю как копию. Хотя в некоторых местах их можно было бы и на константы заменить, но я пока только учусь)

По поводу error - пожалуй да, только вот регистр смешивать мне кажется плохой идеей - лучше уж нижнее подчёркивание. Но переименовать - да.

По количеству комментариев - вроде почистил, хотя, боюсь, их теперь мало, наоборот.

Комментарии к классам и функциям лучше писать в стиле doxygen.

> А тут автор сам просит критики и реагирует на неё адекватно.

Он *не* реагирует.

В прошлый раз, когда из его лапши хоть что-то можно было понять, я ему сказал, как примерно раскинуть его код по классам, чтобы было читамо. Ноль внимания, наоборот, придумал какие-то запутывающие классы.

К сожалению — его место чистить сортиры.

>В прошлый раз, когда из его лапши хоть что-то можно было понять, я ему сказал, как примерно раскинуть его код по классам

можно ссылку где было это?

Спасибо, почитаю.

Нашёл:

(примерно: сделать класс на каждую итерацию, внеся туда все, что зависит только от i, а что зависит еще от i-1 вынести в отдельный массив)

Можно подробней немного?

Относительно того, что есть сейчас - понятно, что это совсем не то, чего хотелось бы получить в итоге.

Раздумываю относительно более удобной структуры. Что-то в духе:

#include <vector>

/*
 * the main class, containing the whole project. Is anything needed here at all?
 */
class project {
  
};

/*
 * class, containing basic data for the project.
 */

class data: public project {
  std::vector <long double> cf_invest;
  std::vector <long double> cf_revenue;
  int length;
  double percent;
public:
  data (); // data input
  long double invest_out (int step);
  long double revenue_out (int step);
  int length_out ();
  double percent_out ();
};

/*
 * class, containing middle calculations
 */

class calculations: public project {
  std::vector <long double> k_discont;
  std::vector <long double> k_inflate;
  std::vector <long double> discont_cf_invest;
  std::vector <long double> discont_cf_revenue;
  std::vector <long double> inflate_cf_revenue;
  std::vector <long double> cf_total;
  std::vector <long double> cf_accumulate;
public:
  calculations (data input_data);
  long double k_discont_out (int step);
  long double k_inflate_out (int step);
  long double discont_cf_invest_out (int step);
  long double discont_cf_revenue_out (int step);
  long double inflate_cf_revenue_out (int step);
  long double cf_total_out (int step);
  long double cf_accumulate_out (int step);
};

/*
 * Class that outputs important project's characteristicks
 */

class output: public project {
  long double profit_index (data input_data, calculations calc_data);
  long double npv (data input_data, calculations calc_data);
  float profit_period (data input_data, calculations calc_data);
  long double ea_calc (data input_data, calculations calc_data);
  long double mirr_calc (data input_data, calculations calc_data);
  long double irr_calc (data input_data, calculations calc_data);
};

Не уверен, что есть необходимость делать более мелкое дробление, так как в функциях будет мало общего кода.

to CyberTribe:

если ты будешь дописывать новый код, не приведя в порядок старый, то результат будет ухудшаться

прошлый твой код был быдлокодом, этот — хуже, чем быдлокод (Led был прав)

хочешь учиться — приводи код в *полный* порядок (а фактически это означает полное переписывание, возможно несколько раз)

_________________________________________________

to ALL:

где можно найти список стандартных ошибок, чтобы выдавать ему их номера у каждой строчки, а не тратить время на объяснения?

типа

N17 = «лишнее сокращение в названии переменной — переписать название переменной, не сокращая слова»

N18 = «рашн инглиш в названии переменной — переставить определение и определяемое местами»

N19 = «переменную, которая возвращается из функции, надо назвать result или res» ?

Я не новый код буду дописывать, а полностью переписывать старый. Так что тут всё нормально. Нового я ничего не добавил пока даже с первого раза. Полностью переписывать я не против - учиться никогда не поздно и вообще полезно)

Приведённый пример заголовка - это то как я сейчас думаю реорганизовать код.

Strochki 34-49 - vinesi v otdelnuyu funcziu int readProjectLength(int minimumSize) - togda ne nuzhen budet komentarii

a potom vizivai #define MINIMUM_PROJECT_LENGTH 0 ... projectLength = readProjectLength(MINIMUM_PROJECT_LENGTH); ...

To zhe samoe 51-68.. Dalshe ne smotrel, ibo code trudno chitaem

еще проще вариант:

/// все одногодовые показатели, что спрашиваются у юзера
class UserInput {
public:
  UserInput() { std::cin >> ...... }
  const double foo;
  const double bar;
};

class YearIndicators { 
public:
  YearIndicators(UserInput user_input, YearIndicators previous_year) {...}
  const double k_discont; 
  const double k_inflate; 
  const double discont_cf_invest; 
  const double discont_cf_revenue; 
  const double inflate_cf_revenue; 
  const double cf_total; 
  const double cf_accumulate;
};

все что не влезет в эту простую схему (IRR approximation?), попытаться тоже упростить, но пока выбросить

да, и сделать перемнным нормальные несокращенные названия — всем похрен, что «тем кто в теме они ясны».

UserInput — это данные за один год, так что лучше м.б. OneYearUserInput

надеюсь, идея ясна

По названиям переменных - уж больно длинными они выйдут, но в принципе не суть важно.

по варианту пара вопросов сразу:
предлагается классовые переменные делать публичными?
И не понятно почему используется const там.

В принципе вариант простой, но при добавлении расчёта IRR придётся полностью переделывать. Впрочем, лучше медленно но правильно.

так понятней. в целом идея ясна. тогда классовая функция будет в цикле вызываться, уже в самом теле программы?

Вообще, джаббер есть? - там вопросы удобней задавать.

Если есть желание помочь, конечно.

понятно, что полей в классе м.б. больше, но никаких xxx_calc()

Там и невооружённым глазом видно, что автору есть над чем работать. Думаю со временем стиль сложится

Какой может быть стиль, когда отсутствует элементарная логика:

do {
error = false;
<Действия1>
if (<Условие1>) {
<Действия2>
error = false;
}
} while (error == true);

Когда элементарно, логично и достаточно:

do {
<Действия1>
} while (!<Условие1>);
<Действия2>

И ты хочешь сказать, что первый - «более понятный», чем второй?

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

постучался к тебе в fedor.duhovskoy@gmail.com

но учти — помогать буду очень краткими репликами

(а сегодня я скоро спать лягу)

Если мы про один и тот же фрагмент, то:

  do
  {
	error = false;
	cout << "Input the length of project" << endl;
	cin >> length;

	// Length can't be less then or equal zero
	if ( length <= 0 ) {
	  cerr << "Wrong length of project" << endl;
	  cerr << "The length of project must be greater then 0" << endl;
	  error = true;
	}
  }
  while (error == true);

Переделать в предложенное вами:

  do
  {
	cout << "Input the length of project" << endl;
	cin >> length;
  }
  while (length <= 0);

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

  do
  {
	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; 
   } 
  }
  while (length <= 0);

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