[C++][быдлокод] Помаленьку учу плюсы.

Соболезную, реально нечитаемый быдлокод в плохом стиле «за 21 день»...

Для начала научись пользоваться функциями, и бросай манеру использовать глобальные переменные :)... Такое ощущение что ты пишешь на каком-то бейсик... Впрочем так и на бейсике не напишешь...

1. оформление ужасно - учись этому сразу, за пример можно взять, например, google code style или любой другой популярный
2. вместо push_back в цикле можно вызвать один insert
3. используй += и т.п.
4. i == 0 ? k_discont[i] = 1 : k_discont[i] = k_discont[i-1] / (1 + (discont));

надо

k_discont[ i ] = !i ? ....

дальше не стал смотреть - не люблю разбираться в каше из букв, да еще и без подсветки синтаксиса :)

>percent = percent/100;
percent /= 100;

Минимизируй глобально используемые данные. Итераор можно объявлять в for цикле:

 for( int i = 0; i <= MAX_ITERATIONS; i++ )
      do_something( i );

Быдлокод, воистину. Сочувствую.

Да я знаю.
Вопрос в том, что с ним сделать, чтобы оно перестало быть таковым?

Почитать умных книжек, например Голуба.
Посмотреть чужой код, желательно побольше кода.

Вопрос в том, что с ним сделать, чтобы оно перестало быть таковым?

для начала напиши комментарии - что за файл, какие в нем функции и для чего. Прокомментируй функции - что делает, входные параметры, что возвращает. Переменные объявлять по одной на строку с описанием назначения переменной. Если не можешь на английском, то напиши на русском. Читать и разбираться в том, что по ссылке никакого желания нет.

>Если не можешь на английском, то напиши на русском.

Но лучше все-таки на аглицком, правило хорошего тона, так сказать.

А то, что там у тебя - вообще разбираться нат желания.

На тебе пример более менее правильно оформленного кода, читабельного хотя бы (создаю главное окно программы на Qt)

#include <QtGui>
#include <QLabel>
#include <QMainWindow>
#include <QWidget>
#include "mainElCat.h"
#include <QAction>
#include "addDialog.h"


//Main GUI window

mainElCat::mainElCat() : QMainWindow()
{
    setWindowTitle("ElCat - Electrical Catalog by Zhbert");

    QWidget *wd = new QWidget(this); //Creating GUI

    //Make the main layout
    QVBoxLayout *mainLay = new QVBoxLayout(this);

    //Create menu
    QAction *aboutAction = new QAction("&About program", this);

    QMenu *fileMenu = menuBar()->addMenu("File");
    fileMenu->addAction("Exit",this,SLOT(exitMenu()));

    QMenu *helpMenu = menuBar()->addMenu("Help");
    helpMenu->addAction(aboutAction);


    //Create fast buttons
    QPushButton *addNewButton = new QPushButton("Add New",this);
    QPushButton *removeItemButton = new QPushButton("Remove Item",this);
    QPushButton *exitButton = new QPushButton("Exit",this);
    QPushButton *helpButton = new QPushButton("Help",this);

    //Secondary lay with buttons
    QGridLayout *buttonsLay = new QGridLayout(this);
    buttonsLay->addWidget(addNewButton,0,0);
    buttonsLay->addWidget(removeItemButton,0,1);
    buttonsLay->addWidget(exitButton,0,2);
    buttonsLay->addWidget(helpButton,0,3);

    //make tooltips of buttons
    addNewButton->setToolTip("Add new item to database");
    removeItemButton->setToolTip("Remove selected item of database");
    exitButton->setToolTip("Exit of program");
    helpButton->setToolTip("View help dialog");

    //Create table
    table = new QTableWidget(0,5);
    table->setGridStyle(Qt::DotLine);
    table->resize(640, 480);
    table->setToolTip("Main table with elements of database");

    QTableWidgetItem *firstHItem = new QTableWidgetItem;
    firstHItem->setText("Num");
    table->setHorizontalHeaderItem(0,firstHItem);
    QTableWidgetItem *secondHItem = new QTableWidgetItem;
    secondHItem->setText("Name of radio-element");
    table->setHorizontalHeaderItem(1,secondHItem);
    QTableWidgetItem *thirdHItem = new QTableWidgetItem;
    thirdHItem->setText("Type of radio-element");
    table->setHorizontalHeaderItem(2,thirdHItem);
    QTableWidgetItem *fourthHItem = new QTableWidgetItem;
    fourthHItem->setText("Number");
    table->setHorizontalHeaderItem(3,fourthHItem);
    QTableWidgetItem *fifthHItem = new QTableWidgetItem;
    fifthHItem->setText("Notes");
    table->setHorizontalHeaderItem(4,fifthHItem);

    table->resizeColumnsToContents();


    connect(exitButton,SIGNAL(clicked()),this, SLOT(exitMenu()));
    connect(addNewButton,SIGNAL(clicked()),this,SLOT(dialodAdd()));



    mainLay->addLayout(buttonsLay);
    mainLay->addWidget(table);


    wd->setLayout(mainLay);
    setCentralWidget(wd);
}

void mainElCat::exitMenu()
{
    QApplication::quit();
}

void mainElCat::dialodAdd()  //Dialog for adding contents to database
{
    int tableindex;
    int i;
    QTableWidgetItem *item;
    QString str;

       dial = new dialogAdd();
       dial->resize(340,220);

       if (dial->exec()==QDialog::Accepted)
       {
            tableindex=table->rowCount();
            tableindex++;
            table->setRowCount(tableindex);

            for (i=1; i<5; i++)
            {
                switch(i)
                {
                case 1:
                    str = dial->name();
                    item = new QTableWidgetItem(str);
                    table->setItem(tableindex-1,i,item);
                    break;
                case 2:
                    str = dial->type();
                    item - new QTableWidgetItem(str);
                    table->setItem(tableindex-1,i,item);
                    break;
                case 3:
                    str = dial->number();
                    item = new QTableWidgetItem(str);
                    table->setItem(tableindex-1,i,item);
                    break;
                case 4:
                    str = dial->notes();
                    item = new QTableWidgetItem(str);
                    table->setItem(tableindex-1,i,item);
                    break;
                default:
                    break;
                }
            }

            table->resizeColumnsToContents();
            table->resizeRowsToContents();
       }
       else
       {
       }
}

Массово используемые глобальные переменные — путь в психушку при отладке. Каждая функция должна иметь контролируемый вход и выход.

А C++ в коде нужен только для использования stl? Может, есть смысл подумать о проектировании класса?

К классу приду чуть позднее.
Но вообще да. Все эти функции надо будет вынести в класс.

Благодарю.

как по мне тоже каша :) вот пример кода из моего велосипеда( который я надеюсь скоро уже выложить как первую альфу ):

http://pastebin.com/tVVh0ast

Ну он у тебя оформлен почти как у меня, только комментариев побольше и разделено более четко.

На тебе пример более менее правильно оформленного кода

Тоже так себе, если честно.

> вот пример кода

за количество строк доплачивают и пустые тоже считают?

>Тоже так себе, если честно.

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

ну тогда пройдусь по твоему коду :)

1.

#include <QtGui> 
#include <QLabel> 
#include <QMainWindow> 
#include <QWidget> 
#include "mainElCat.h" 
#include <QAction> 
#include "addDialog.h"

некрасиво мешать хедера в кучу + надо сортировать по имени

// Qt
#include <QtGui> 
#include <QAction> 
#include <QLabel> 
#include <QMainWindow> 
#include <QWidget> 

// ELCAT
#include "addDialog.h"
#include "mainElCat.h" 

2.

mainElCat::mainElCat() : QMainWindow()

я б записал как

/**********************************************************************************************/
mainElCat::mainElCat( void )
:
   QMainWindow()

но не запишу :) - т.к. вызов дефолтного конструктора руками не нужен

3.

buttonsLay->addWidget(addNewButton,0,0); 

не надо жалеть пробелы - они помогают визуально разделить части кода

4.

    QTableWidgetItem *firstHItem = new QTableWidgetItem; 
    firstHItem->setText("Num"); 
    table->setHorizontalHeaderItem(0,firstHItem); 
    QTableWidgetItem *secondHItem = new QTableWidgetItem; 
    secondHItem->setText("Name of radio-element"); 
    table->setHorizontalHeaderItem(1,secondHItem); 
    QTableWidgetItem *thirdHItem = new QTableWidgetItem; 
    thirdHItem->setText("Type of radio-element"); 
    table->setHorizontalHeaderItem(2,thirdHItem); 
    QTableWidgetItem *fourthHItem = new QTableWidgetItem; 
    fourthHItem->setText("Number"); 
    table->setHorizontalHeaderItem(3,fourthHItem); 
    QTableWidgetItem *fifthHItem = new QTableWidgetItem; 
    fifthHItem->setText("Notes"); 
    table->setHorizontalHeaderItem(4,fifthHItem); 

набор букв :)

> за количество строк доплачивают и пустые тоже считают?

это мой личный код

Ну я потом причесывать его буду, как до релиза дойду. Поначалу, пока мысль в голове вертится, пишется абы как все, лишь бы работало =)

но мне и так понятно что и где.

С такой аргументацией можно что угодно оправдать.

> это мой личный код

а зачем после каждой строчки кода пустая строка?

>а зачем после каждой строчки кода пустая строка?

Мб у ТСа гигантофобия и он боится всего большого, в том числе и больших кусков кода из нескольких строчек кода подряд?

> Мб у ТСа гигантофобия

это ж не ТС )))

> а зачем после каждой строчки кода пустая строка?

это тебе видно с непривычки так кажется( наверное лепишь все сплошным текстом :) ) - в LDCPlayer::LoadData, например, из 51 показанной строки - 8 пустых, которые нужны, чтоб комментарии не «влипли» в код и было видно к чему они относятся

ну тогда жду «правильного» оформления последней функции - интересно ж как надо экономить на строках без потери читабельности

интересно ж как надо экономить на строках без потери читабельности

Как по мне, так читабельность сохраняется правильно подсветкой синтаксиса.

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

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

> интересно ж как надо экономить на строках без потери читабельности

пустые строки мало влияют на читабельность, есть более полезные приёмы. В твоём случае можно тупо поскипать почти все пустые строки и на читабельности это не скажется, если даже не улучшит её. Код должен всё-таки идти блоками, а у тебя он как будто растянут.

и таки будь добр - покажи как надо

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

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

Вот и я так считаю, что удобно, но ахонимус переусердствовал. Применительно к его запросу примера — я бы switch переписал с отступами вместо пробелов и, возможно, саму функцию вообще декомпозировал, слишком длинно выглядит с учётом того, что вся не влезла.

> Применительно к его запросу примера — я бы switch переписал с отступами вместо пробелов

ну покажи уже :)

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

там дальше осталось только около 7-8 элементов switch, а «длинно выглядит» из-за комментариев, если их убрать - кода останется совсем немного, предлагаешь вынести комментарии в отдельный метод?

Я switсh и не дочитал :)

Я бы так сделал

switch( *( start + 1 ) )
{
  // "/brush"
  case 'b':
     mItems += ReadBrushItem( ch, end );
     break;
  // "/clip"
  case 'c':
     mItems += ReadClipItem( ch, end );
     break;
  ...
}

> Я бы так сделал

о ужас - ты решил повторить за мной и написал все 10 case-ов в одной функции, да еще на каждый потратил по 4-ре строки! надо срочно декомпозировать! (с)

о ужас - ты решил повторить за мной и написал все 10 case-ов в одной функции, да еще на каждый потратил по 4-ре строки!

4-5 строк не так и важно, важнее читабельность. Ты посмотри чужие исходники, там объявления то функций по несколько строк (переменнаю на строку).
В общем у каждого свои фломастеры.

после ваших комментариев переделал многое.
Добавил гораздо больше комментов в текст.

http://codepad.org/7JDyYS3U (на этот раз сервис умеет подсветку синтаксиса)

Ругайте. А то как иначе научится писать правильно?

Да, вот про += и подобное забываю.
Надо приучиться делать так.
В остальном?

> после ваших комментариев переделал многое.

- если используешь глобальные переменные - то используй разный стиль для их имен( например префикс g ), но лучше завести функцию - чтоб избежать «неконтролируемой» инициализации
- с комментариями ты перестарался
- k_discont[i]
- push_back
- добавляй «разделитель» перед каждой функцией, чтоб при листании кода было сразу видно, когда начинается новая

ну и:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

я хоть и не читал все, но думаю гугль должен иметь хороший стиль

using std::cin; вместо using namespace - хорошо.

глобальные переменные - очень плохо. Их не должно быть вообще.

опять-же объявлять переменные в одну строчку - нечитабельно.

  // Initialization of vectors
  for (i=0;i<=period;++i) {
    invest.push_back (0);
...

Кошмар. Можно просто invest.resize(period+1). А если все эти векторы будут членами класса, но ещё лучше:

bla_bla_bla::bla_bla_bla(int period)
  :invest(period+1)
  ,revenue(period+1)
  ...

  if (percent <= 0 || percent >= 1) {
    cerr << "Wrong rate of discont" << endl;
//тут должен быть return или throw

  for (i = 0; i <= period; ++i) {

Во-первых, счётчик цикла не можно, а нужно объявлять прямо в тут, во-вторых как индекс массива лучше беззнаковый тип, оптимально - size_t. Хотя я люблю итераторы. А, да, ещё: вот это «меньше или равно» - это нетипично. Если бы это был код опытного программиста - я бы счёл, что за «<=» скрывается Существенная Деталь. В случае с новичком я не уверен. Советую убедиться, что это важно либо переписать как у всех - «for(size_t i=0;i<i_max;++i)»

void profit_period ()
{
  // calculating the period for getting profit
  for (i = 0; i <= period; ++i) {
    if (d_total[i] >= 0) {
      p_profit = (i - 1) + (-d_total[i-1]/d_float[i]);
      }
  }
}

Что будет, если (d_total[0] >= 0)? Если такого не может быть - напиши ассерт. Если функция рассчитывает, что чего-то не может быть - это надо явно указывать ассертом. При отладке экономит время - аж дух захватывает.

  for (int j = 1; j <= 10; ++j) {

Почему 10? Магические числа в топку. как константы в коде могут быть 0, 1, 60*60*24 (если действие не очень важное, типа установки кукисов в HTTP).

> // calculating index of profit

for ( int i = 0 ; i <= length ; ++i ) {

d_invest_sum += d_invest[i];

d_revenue_sum += d_revenue[i];

}

лучше использовать accumulate

плюсую ко всему

П.С. пошел добавлять ассерты в старый код :)

>http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

я хоть и не читал все, но думаю гугль должен иметь хороший стиль

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

> Вооще компания, которая не в состоянии нормально сделать хром и пикасу для линукса не может использоваться как образец.

это скорее говорит о том, что линукс для них не приоритетен

Не вкурил: а в чем глубокий философский смысл в for + switch? На каждой итерации цикла будет выполняться case i, почему бы вообще не убрать цикл со switсh и выполнять код из case'ов?

item - new QTableWidgetItem(str) - явно опечатка.

Про глобальные переменные понятно. Надо будет переписать всё с использованием итераторов и убрать в класс, тогда можно будет избавиться от глобальных векторов.

Про invest.resize(period+1) спасибо. Как раз думал как это сделать нормально. Понятно, что в классе они будут сразу нужного размера создаваться.

for (i = 0; i <= period; ++i) {

тут либо <= либо к period прибавлять 1. <= мне кажется удобней.

(d_total[0] >= 0)

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

for (int j = 1; j <= 10; ++j) {

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

>Надо будет переписать всё с использованием итераторов

Итераторы - это не обязательно. Дело вкуса. Без auto из нового стандарта ими несколько неудобно пользоваться. Но с итераторами на шаг ближе к std::accumulate и подобным зайчаткам функциональщины.

>тут либо <= либо к period прибавлять 1

1 можно добавить сразу после ввода.

>В принципе такое может быть, если пользователь ошибается при вводе данных

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

>Ну и прочитать про ассерты и сделать.

Ассерты - это просто.

#include<cassert>
//...
double sqrt(double val)
{
assert(val>=0);
//...

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

int i;
cin >> i;
assert( i>0 ); //плохо
string s;
getline(s,cin);
assert(file_exist(s)); //плохо

Это будет профанация идеи. Во-первых, на ассерте программа будет либо полностью падать, либо (в релизной сборке с -DNDEBUG) ассерт вытрется из кода препроцессором.

Ещё могу посоветовать пользоваться типизацией. Если аргумент функции неотрицательное целове - то и писать надо unsigned вместо int.

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

Вот половину коментов, которые ты дописал в новом варианте можно выкинуть и написать комент про 10 (вынеся её в константу в начале файла) - и это будет улучшением.