crossplatform.ru

Здравствуйте, гость ( Вход | Регистрация )

AD
  опции профиля:
сообщение 5.8.2008, 16:26
Сообщение #1


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


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

Если разрешите, то могу начать.
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
3 страниц V   1 2 3 >  
Начать новую тему
Ответов (1 - 26)
void*
  опции профиля:
сообщение 5.8.2008, 18:46
Сообщение #2


Программист-самоучка
***

Группа: Участник
Сообщений: 429
Регистрация: 4.6.2008
Пользователь №: 193

Спасибо сказали: 28 раз(а)




Репутация:   3  


ну вот у меня такая дилема. Нижеприведенный код показывает в дереве (QTreeWidget'e) содержимое определенной папки методом рекурсии, однако состоит это дело из двух функций, и мне почему-то кажется что можно это как-то сделать и через одну функцию:
код
void ProjectView::createItems(const QDir &dir) { //параметром является нужная директория

     QStringList listDirs = dir.entryList(QDir::Dirs);

     foreach(QString dirname, listDirs) {
                               if(dirname == "." || dirname == "..") continue;
                               curItem = new QTreeWidgetItem(rootItem); //curItem и rootItem объявлены в классе ранее
                               curItem->setText(0, dirname);                    //rootItem является главным итемом, к нему 
                               curItem->setIcon(0, folderIcon);                 //присоединяются все остальные
                               startDir(QDir(dir.absoluteFilePath(dirname)), curItem); //curItem -  просто единый указатель 
     }                                                                                     //использующийся для создания всех итемов


     QStringList listFiles = dir.entryList(QDir::Files);

     foreach(QString file, listFiles) {
                               curItem = new QTreeWidgetItem(rootItem);
                               curItem->setData(0, Qt::UserRole, QVariant(dir.absolutePath() + "/" + file));
                               curItem->setText(0, file);
                               curItem->setText(1, dir.absoluteFilePath(file));
                               curItem->setIcon(0, fileIcon);
                               //curItem->setFlags(...);
     }
}
void ProjectView::startDir(const QDir& dir, QTreeWidgetItem *parent) { //рекурсивная функция
     QStringList listFiles = dir.entryList(QDir::Files);
     foreach(QString filename, listFiles) {
                               curItem = new QTreeWidgetItem(parent);
                               curItem->setData(0, Qt::UserRole, QVariant(dir.absolutePath() + "/" + filename));
                               curItem->setText(0, filename);
                               curItem->setText(1, dir.absoluteFilePath(filename));
                               curItem->setIcon(0, fileIcon);
     }

     QStringList listDirs = dir.entryList(QDir::Dirs);
     foreach(QString subdir, listDirs) {
                               if(subdir == "." || subdir == "..") continue;
                               curItem = new QTreeWidgetItem(parent);
                               curItem->setText(0, subdir);
                               curItem->setIcon(0, folderIcon);
                               startDir(QDir(dir.absoluteFilePath(subdir)), curItem);
     }
}
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
ViGOur
  опции профиля:
сообщение 5.8.2008, 21:59
Сообщение #3


Мастер
******

Группа: Модератор
Сообщений: 3296
Регистрация: 9.10.2007
Из: Москва
Пользователь №: 4

Спасибо сказали: 231 раз(а)




Репутация:   40  


Ну я дуаю ты сам мог бы догадаться, если бы был внимательней.
Не нужно два цикла, вместо entryList используй entryInfoList в и foreach получай QFileInfo с помощью которого ты можешь узнать файл это или директория (isFile или isDir).

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

p.s. как сделаешь код здесь выложи, чтобы покритиковали если будет что. :)
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
void*
  опции профиля:
сообщение 5.8.2008, 22:21
Сообщение #4


Программист-самоучка
***

Группа: Участник
Сообщений: 429
Регистрация: 4.6.2008
Пользователь №: 193

Спасибо сказали: 28 раз(а)




Репутация:   3  


ViGOur, насчет entryInfoList я знал, просто мне казалось что так нагляднее. А так - пока что повозиться с этим кодом нет времени и свободных рук :) просто для начала сделал попроще и нагляднее, а потом уже можно будет как-нибудь оптимизировать.
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Red Devil
  опции профиля:
сообщение 6.8.2008, 11:59
Сообщение #5


Студент
*

Группа: Участник
Сообщений: 68
Регистрация: 6.6.2008
Из: Saint-Petersburg
Пользователь №: 194

Спасибо сказали: 1 раз(а)




Репутация:   3  


void ProjectView::AddDirs(const QDir & Dir, QTreeWidget * pParent)
{
    QStringList DirsList = Dir.entryList(QDir::Dirs);
    foreach(QString Dirname, DirsList) 
    {
        if(Dirname == "." || Dirname == "..") 
            continue;
      
        QTreeWidget * pItem = new QTreeWidgetItem(pParent);               
        pItem->setText(0, dirname);                          
        pItem->setIcon(0, folderIcon);                        
        StartDir(QDir(dir.absoluteFilePath(dirname)), pItem);  
    }                                                                                 
}

void ProjectView::AddFiles(const QDir & Dir, QTreeWidgetItem * pParent)
{
    QStringList FilesList = Dir.entryList(QDir::Files);

    foreach(QString File, FilesList) 
    {
        QTreeWidget * pItem = new QTreeWidgetItem(pParent);
        pItem->setData(0, Qt::UserRole, QVariant(Dir.absolutePath() + "/" + File));
        pItem->setText(0, File);
        pItem->setText(1, Dir.absoluteFilePath(file));
        pItem->setIcon(0, fileIcon);
    }
}

void ProjectView::CreateItems(const QDir & Dir) 
{
    AddDirs(Dir, rootItem);
    AddFiles(Dir, rootItem);
}

void ProjectView::StartDir(const QDir & Dir, QTreeWidgetItem * pParent)
{
    AddFiles(Dir, pParent);
    AddDirs(Dir, pParent);
}


Сообщение отредактировал Red Devil - 6.8.2008, 12:01
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
void*
  опции профиля:
сообщение 6.8.2008, 12:49
Сообщение #6


Программист-самоучка
***

Группа: Участник
Сообщений: 429
Регистрация: 4.6.2008
Пользователь №: 193

Спасибо сказали: 28 раз(а)




Репутация:   3  


упростил однако :) из двух функций сделал четыре :)
хотя так оно конечно возможно понятнее
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
ViGOur
  опции профиля:
сообщение 6.8.2008, 14:22
Сообщение #7


Мастер
******

Группа: Модератор
Сообщений: 3296
Регистрация: 9.10.2007
Из: Москва
Пользователь №: 4

Спасибо сказали: 231 раз(а)




Репутация:   40  


void createItems(const QDir &dir, QTreeWidgetItem *parent = 0)
{ 

    QFileInfoList fiList = dir.entryInfoList();

    foreach(QFileInfo fi, fiList) 
    {
        QTreeWidgetItem *curItem = 0;
        if( fi.isDir())
        {
            QString szDirName = fi.fileName();
            if( szDirName=="." || szDirName=="..")
                continue;

            curItem = new QTreeWidgetItem(parent);
            curItem->setText(0, szDirName);
            curItem->setIcon(0, folderIcon);
            
            createItems( fi.absoluteFilePath(), curItem);
        }
        else if( fi.isFile())
        {
            QString szFileName = fi.fileName();
            curItem = new QTreeWidgetItem(parent);
            curItem->setData(0, Qt::UserRole, QVariant(dir.absolutePath() + "/" + szFileName));
            curItem->setText(0, szFileName);
            curItem->setText(1, dir.absoluteFilePath(szFileName));
            curItem->setIcon(0, fileIcon);
        }
    }
}


Вызывается так:
createItems( QDir( "c:\\temp\\"), rootItem);
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
void*
  опции профиля:
сообщение 6.8.2008, 17:26
Сообщение #8


Программист-самоучка
***

Группа: Участник
Сообщений: 429
Регистрация: 4.6.2008
Пользователь №: 193

Спасибо сказали: 28 раз(а)




Репутация:   3  


ViGOur, спасибо! примерно такое у меня и вертелось в голове, только никак не мог выразить это в коде :)
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Tonal
  опции профиля:
сообщение 7.8.2008, 8:09
Сообщение #9


Активный участник
***

Группа: Участник
Сообщений: 452
Регистрация: 6.12.2007
Из: Новосибирск
Пользователь №: 34

Спасибо сказали: 69 раз(а)




Репутация:   17  


2 ViGOur
Я бы всё таки выделил из этого кода функции создания итема для файла и для директории. Тогда код получается максимально наглядный и все занимаются своим делом:
QTreeWidgetItem* createDirItem(QTreeWidgetItem& parent, const QFileInfo& fi);
QTreeWidgetItem* createFileItem(QTreeWidgetItem& parent, const QFileInfo& fi);
void createItems(const QDir &dir, QTreeWidgetItem *parent = 0) { 

    QFileInfoList fiList = dir.entryInfoList();

    foreach(QFileInfo fi, fiList) 
    {
        QTreeWidgetItem *curItem = 0;
        if( fi.isDir())
        {
            QString szDirName = fi.fileName();
            if( szDirName=="." || szDirName=="..")
                continue;

            createItems(fi.absoluteFilePath(), createDirItem(*parent, fi));
        }
        else if( fi.isFile())
        {
            createFileItem(*parent, fi);
        }
    }
}


Сообщение отредактировал Tonal - 7.8.2008, 8:12
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
ViGOur
  опции профиля:
сообщение 7.8.2008, 8:27
Сообщение #10


Мастер
******

Группа: Модератор
Сообщений: 3296
Регистрация: 9.10.2007
Из: Москва
Пользователь №: 4

Спасибо сказали: 231 раз(а)




Репутация:   40  


Цитата(Tonal @ 7.8.2008, 9:09) *
Я бы всё таки выделил из этого кода функции создания итема для файла и для директории
Согласен, так как могут понадобится дополнительные манипуляции с папками и файлами.
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
AD
  опции профиля:
сообщение 7.8.2008, 9:10
Сообщение #11


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


Выдалась возможность, может сможете и мне упростить функцию?


Вот ее код:

/// Класс экранных траекторий
class ScrTrack: public TImageClass
{
private:
    std::string ini_file_name;            ///< имя ини-файла
    int readed_time;                    ///< время для разрыва траектории, прочтенное из файла
    int readed_dist;                    ///< расстояние для разрыва траектории, прочтенное из файла
    bool event_picture;                    ///< флажок, предупреждающий, нажата ли кнопка отображения событий
    bool event_name_press;                ///< флажок, предупреждающий, нажата ли кнопка отображения названий событий

protected:
    std::vector<GEOPOINT>* geo;            ///< указатель на вектор с географическими координатами

public:
    enum GAPTYPE { GT_NONE = 0, GT_TIMEGAP, GT_DISTGAP, GT_TMDSTGAP };
    SubSahara* pSahara;                    ///< указатель на Сахару
    bool init_gaps;                        ///< флажок, указывающий когда следует пересчитывать данные для графиков

public:
    ScrTrack(SubSahara* pSah): readed_time(0), readed_dist(0), geo(NULL), pSahara(pSah), init_gaps(false),
                    bmpSymbol(NULL), event_name_press(false), event_picture(false) {}
    virtual ~ScrTrack() {}
    virtual void DrawClass(GRAPHIC_DEVICE& device, const CHART_SCOPE& scope, const DRAW_MODE& mode);
    virtual void LoadSymbols();
    virtual void UnloadSymbols();
    
    bool enumPossible(std::vector<GEOPOINT>::iterator cur, std::vector<GEOPOINT>::iterator next);
};


/// Перебор возможных вариантов разрывов и их обработка
bool ScrTrack::enumPossible(vector<GEOPOINT>::iterator cur, vector<GEOPOINT>::iterator next)
{
    ScrTrack::GAPTYPE gapType = checkGaps();
    time_t time_diff;                        double pos_diff;
    bool isContinue = true;
    switch(gapType)
    {
    case ScrTrack::GT_NONE:
        readed_time = 10;
    break;
    case ScrTrack::GT_TIMEGAP:
        if(next -> status_lat == PS_NONE || next -> status_lon == PS_NONE || cur -> status_lat == PS_NONE
            || cur -> status_lon == PS_NONE)
            break;
        time_diff = (time_t)fabs(double(next -> time_marker - cur -> time_marker));
        if(time_diff > readed_time) isContinue = false;
        if(isContinue && init_gaps)
        {
            if(next -> status_lat != PS_FAIL && next -> status_lon != PS_FAIL && cur -> status_lat != PS_FAIL
                && cur -> status_lon != PS_FAIL && next -> status_lat != PS_NODATA && next -> status_lon != PS_NODATA
                && cur -> status_lat != PS_NODATA && cur -> status_lon != PS_NODATA)
                q_calc.maxTime += time_diff;
            PARAMVALUE val;
            val.value = (double)time_diff;                val.status = next -> status_lat;
            x_data.time_x.push_back(val);
        }
    break;
    case ScrTrack::GT_DISTGAP:
        if(next -> status_lat == PS_NONE || next -> status_lon == PS_NONE || cur -> status_lat == PS_NONE
            || cur -> status_lon == PS_NONE)
            break;
        pos_diff = diffDist(cur, next);
        if(pos_diff > readed_dist) isContinue = false;
        if(isContinue && init_gaps)
        {
            if(next -> status_lat != PS_FAIL && next -> status_lon != PS_FAIL && cur -> status_lat != PS_FAIL
                && cur -> status_lon != PS_FAIL && next -> status_lat != PS_NODATA && next -> status_lon != PS_NODATA
                && cur -> status_lat != PS_NODATA && cur -> status_lon != PS_NODATA)
                q_calc.maxDist += pos_diff;
            PARAMVALUE val;
            val.value = pos_diff;                val.status = next -> status_lat;
            x_data.dist_x.push_back(val);
        }
    break;
    case ScrTrack::GT_TMDSTGAP:
        if(next -> status_lat == PS_NONE || next -> status_lon == PS_NONE || cur -> status_lat == PS_NONE
            || cur -> status_lon == PS_NONE)
            break;
        time_diff = (time_t)fabs(double(next -> time_marker - cur -> time_marker));
        pos_diff = diffDist(cur, next);
        if(time_diff > readed_time || pos_diff > readed_dist) isContinue = false;
        if(isContinue && init_gaps)
        {
            if(next -> status_lat != PS_FAIL && next -> status_lon != PS_FAIL && cur -> status_lat != PS_FAIL
                && cur -> status_lon != PS_FAIL && next -> status_lat != PS_NODATA && next -> status_lon != PS_NODATA
                && cur -> status_lat != PS_NODATA && cur -> status_lon != PS_NODATA)
            {
                q_calc.maxDist += pos_diff;
                q_calc.maxTime += time_diff;
            }
            PARAMVALUE val;
            val.value = pos_diff;                val.status = next -> status_lat;
            x_data.dist_x.push_back(val);
            val.value = (double)time_diff;
            x_data.time_x.push_back(val);
        }
    break;
    }

    return isContinue;
}



Для того, чтобы было чуть понятнее привел кусочек класса, где определена эта функция.
Ну что можно сказать: функция выполняет расчет и добавление в вектор значений в зависимости от значения GAPTYPE! Я подумывал о том, чтобы вынести расчет для каждого case в отдельную функцию, но посчитал количество параметров этой функции и стало грустно! Может сможете улучшить данный код? Просьба приводить пример улучшения кода. Заранее спасибо!
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Tonal
  опции профиля:
сообщение 7.8.2008, 12:02
Сообщение #12


Активный участник
***

Группа: Участник
Сообщений: 452
Регистрация: 6.12.2007
Из: Новосибирск
Пользователь №: 34

Спасибо сказали: 69 раз(а)




Репутация:   17  


Вынеси длинные проверки с next -> status... в отдельные функции - сразу жить станет легче, и станит видно что делать дальше. :)
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
AD
  опции профиля:
сообщение 7.8.2008, 12:55
Сообщение #13


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


Цитата(Tonal @ 7.8.2008, 13:02) *
Вынеси длинные проверки с next -> status... в отдельные функции - сразу жить станет легче, и станит видно что делать дальше. :)

Если честно, наглядность не сильно увеличилась:
Исходник

/// Класс экранных траекторий
class ScrTrack: public TImageClass
{
private:
    std::string ini_file_name;            ///< имя ини-файла
    int readed_time;                    ///< время для разрыва траектории, прочтенное из файла
    int readed_dist;                    ///< расстояние для разрыва траектории, прочтенное из файла
    bool event_picture;                    ///< флажок, предупреждающий, нажата ли кнопка отображения событий
    bool event_name_press;                ///< флажок, предупреждающий, нажата ли кнопка отображения названий событий

protected:
    std::vector<GEOPOINT>* geo;            ///< указатель на вектор с географическими координатами

public:
    enum GAPTYPE { GT_NONE = 0, GT_TIMEGAP, GT_DISTGAP, GT_TMDSTGAP };
    SubSahara* pSahara;                    ///< указатель на Сахару
    bool init_gaps;                        ///< флажок, указывающий когда следует пересчитывать данные для графиков

private:
    bool checkStatusDist(const GEOPOINT current, const GEOPOINT next);
    bool checkStatusBreak(const GEOPOINT current, const GEOPOINT next);

public:
    ScrTrack(SubSahara* pSah): readed_time(0), readed_dist(0), geo(NULL), pSahara(pSah), init_gaps(false),
                    bmpSymbol(NULL), event_name_press(false), event_picture(false) {}
    virtual ~ScrTrack() {}
    virtual void DrawClass(GRAPHIC_DEVICE& device, const CHART_SCOPE& scope, const DRAW_MODE& mode);
    virtual void LoadSymbols();
    virtual void UnloadSymbols();
    
    bool enumPossible(std::vector<GEOPOINT>::iterator cur, std::vector<GEOPOINT>::iterator next);
};


/// Проверка статусов координат и времени для подсчета расстояния (временного или же километрового)
bool ScrTrack::checkStatusDist(const GEOPOINT current, const GEOPOINT next)
{
    bool f = false;
    if(next.status_lat != PS_FAIL && next.status_lon != PS_FAIL && current.status_lat != PS_FAIL
         && current.status_lon != PS_FAIL && next.status_lat != PS_NODATA && next.status_lon != PS_NODATA
         && current.status_lat != PS_NODATA && current.status_lon != PS_NODATA)
        f = true;

    return f;
}

/// Проверка статусов координат для метки о том, что из цикла надо выйти
bool ScrTrack::checkStatusBreak(const GEOPOINT current, const GEOPOINT next)
{
    bool f = false;
    if(next.status_lat == PS_NONE || next.status_lon == PS_NONE || current.status_lat == PS_NONE
         || current.status_lon == PS_NONE)
        f = true;

    return f;
}

/// Перебор возможных вариантов разрывов и их обработка
bool ScrTrack::enumPossible(vector<GEOPOINT>::iterator cur, vector<GEOPOINT>::iterator next)
{
    ScrTrack::GAPTYPE gapType = checkGaps();
    time_t time_diff;                        double pos_diff;
    bool isContinue = true;
    switch(gapType)
    {
    case ScrTrack::GT_NONE:
        readed_time = 10;
    break;
    case ScrTrack::GT_TIMEGAP:
        if(checkStatusBreak(*cur, *next))
            break;
        time_diff = (time_t)fabs(double(next -> time_marker - cur -> time_marker));
        if(time_diff > readed_time) isContinue = false;
        if(isContinue && init_gaps)
        {
            if(checkStatusDist(*cur, *next))
                q_calc.maxTime += time_diff;
            PARAMVALUE val;
            val.value = (double)time_diff;                val.status = next -> status_lat;
            x_data.time_x.push_back(val);
        }
    break;
    case ScrTrack::GT_DISTGAP:
        if(checkStatusBreak(*cur, *next))
            break;
        pos_diff = diffDist(cur, next);
        if(pos_diff > readed_dist) isContinue = false;
        if(isContinue && init_gaps)
        {
            if(checkStatusDist(*cur, *next))
                q_calc.maxDist += pos_diff;
            PARAMVALUE val;
            val.value = pos_diff;                val.status = next -> status_lat;
            x_data.dist_x.push_back(val);
        }
    break;
    case ScrTrack::GT_TMDSTGAP:
        if(checkStatusBreak(*cur, *next))
            break;
        time_diff = (time_t)fabs(double(next -> time_marker - cur -> time_marker));
        pos_diff = diffDist(cur, next);
        if(time_diff > readed_time || pos_diff > readed_dist) isContinue = false;
        if(isContinue && init_gaps)
        {
            if(checkStatusDist(*cur, *next))
            {
                q_calc.maxDist += pos_diff;
                q_calc.maxTime += time_diff;
            }
            PARAMVALUE val;
            val.value = pos_diff;                val.status = next -> status_lat;
            x_data.dist_x.push_back(val);
            val.value = (double)time_diff;
            x_data.time_x.push_back(val);
        }
    break;
    }

    return isContinue;
}

Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Tonal
  опции профиля:
сообщение 8.8.2008, 15:21
Сообщение #14


Активный участник
***

Группа: Участник
Сообщений: 452
Регистрация: 6.12.2007
Из: Новосибирск
Пользователь №: 34

Спасибо сказали: 69 раз(а)




Репутация:   17  


Без тега кода всяко только уменьшилась! :)

По коду:
bool ScrTrack::checkStatusBreak(const GEOPOINT& current, const GEOPOINT& next)
{
  return next.status_lat == PS_NONE || next.status_lon == PS_NONE || current.status_lat == PS_NONE
|| current.status_lon == PS_NONE;
}

Вторую аналогично.

После проверок где isContinue = false и checkStatusBreak сразу выходить.
Проверку на init_gaps в начало функции. и тоже по false сразу выходить.
Изменение, которое после checkStatusDist тоже в 2 отдельные функции.
Ну и из (time_t)fabs(double(next -> time_marker - cur -> time_marker)) тоже функцию.
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
AD
  опции профиля:
сообщение 8.8.2008, 16:38
Сообщение #15


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


Tonal, я не понял как и что. Можно все-таки кодом, пожалуйста!
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
AD
  опции профиля:
сообщение 10.8.2008, 16:59
Сообщение #16


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


А мне помогут упростить код?
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Tonal
  опции профиля:
сообщение 10.8.2008, 17:31
Сообщение #17


Активный участник
***

Группа: Участник
Сообщений: 452
Регистрация: 6.12.2007
Из: Новосибирск
Пользователь №: 34

Спасибо сказали: 69 раз(а)




Репутация:   17  


Чё непонятно-то?
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
AD
  опции профиля:
сообщение 10.8.2008, 22:06
Сообщение #18


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


Цитата(Tonal @ 10.8.2008, 18:31) *
Чё непонятно-то?

непонятно что и как надо сделать!
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Tonal
  опции профиля:
сообщение 11.8.2008, 8:05
Сообщение #19


Активный участник
***

Группа: Участник
Сообщений: 452
Регистрация: 6.12.2007
Из: Новосибирск
Пользователь №: 34

Спасибо сказали: 69 раз(а)




Репутация:   17  


Цитата
Дай - понимаю, курить - понимаю, дай курить - не понимаю!

Какое предложение объяснить?

Да, и функции, вместо checkStatus* я бы назвал isStatus* - во первых сразу понятно что они возвращают и зачем, во вторых соответствует стилю Qt. :)
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
AD
  опции профиля:
сообщение 11.8.2008, 8:48
Сообщение #20


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


Цитата(Tonal @ 11.8.2008, 9:05) *
После проверок где isContinue = false и checkStatusBreak сразу выходить.
Проверку на init_gaps в начало функции. и тоже по false сразу выходить.
Изменение, которое после checkStatusDist тоже в 2 отдельные функции.
Ну и из (time_t)fabs(double(next -> time_marker - cur -> time_marker)) тоже функцию.

Что сделать надо, что именно и куда вынести?
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Tonal
  опции профиля:
сообщение 11.8.2008, 11:07
Сообщение #21


Активный участник
***

Группа: Участник
Сообщений: 452
Регистрация: 6.12.2007
Из: Новосибирск
Пользователь №: 34

Спасибо сказали: 69 раз(а)




Репутация:   17  


1) После проверок где...
Код
if(...) isContinue = false;

заменить на
if(...) 
  return false;

2) Проверку на..
Код
if(isContinue && init_gaps) {

заменить на
if(!init_gaps)
  return true

3) Изменение, которое после checkStatusDist...
У тебя 3 ветки. Если смотреть на код этих веток после if(isContinue && init_gaps)... то ясно что третья ветка - это объединение первых 2. Стало быть мы выносим этот код из 1ий и 2ой ветви в отдельные функции а в третьей ветви вызываем обе этих функции.
4) Ну и из...
Из кода (time_t)fabs(double(next -> time_marker - cur -> time_marker));
Делаем функцию timeDist
5) Функции checkStatus* переименовываем в isStatus*.
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
AD
  опции профиля:
сообщение 11.8.2008, 14:54
Сообщение #22


Профессионал
*****

Группа: Участник
Сообщений: 2003
Регистрация: 4.2.2008
Из: S-Petersburg
Пользователь №: 84

Спасибо сказали: 70 раз(а)




Репутация:   17  


Теперь это выглядит более внятно и легче править. Спасибо!
Source

/// Проверка статусов координат и времени для подсчета расстояния (временного или же километрового)
bool ScrTrack::isStatusDist(const GEOPOINT current, const GEOPOINT next)
{
    bool f = false;
    if(next.status_lat != PS_FAIL && next.status_lon != PS_FAIL && current.status_lat != PS_FAIL
         && current.status_lon != PS_FAIL && next.status_lat != PS_NODATA && next.status_lon != PS_NODATA
         && current.status_lat != PS_NODATA && current.status_lon != PS_NODATA)
        f = true;

    return f;
}

/// Проверка статусов координат для метки о том, что из цикла надо выйти
bool ScrTrack::isStatusBreak(const GEOPOINT current, const GEOPOINT next)
{
    bool f = false;
    if(next.status_lat == PS_NONE || next.status_lon == PS_NONE || current.status_lat == PS_NONE
         || current.status_lon == PS_NONE)
        f = true;

    return f;
}

/// Добавление значения времени в вектор
bool ScrTrack::addTimeValue(vector<GEOPOINT>::iterator cur, vector<GEOPOINT>::iterator next)
{
    if(isStatusBreak(*cur, *next)) return false;
    time_t time_diff = (time_t)fabs(double(next -> time_marker - cur -> time_marker));
    if(time_diff > readed_time) return false;

    if(!init_gaps) return true;
    if(isStatusDist(*cur, *next))
        q_calc.maxTime += time_diff;

    PARAMVALUE val;
    val.value = (double)time_diff;
    val.status = next -> status_lat;
    x_data.time_x.push_back(val);

    return true;
}

/// Добавление значения расстояния в вектор
bool ScrTrack::addDistValue(vector<GEOPOINT>::iterator cur, vector<GEOPOINT>::iterator next)
{
    if(isStatusBreak(*cur, *next)) return false;
    double pos_diff = diffDist(cur, next);
    if(pos_diff > readed_dist) return false;

    if(!init_gaps) return true;
    if(isStatusDist(*cur, *next))
        q_calc.maxDist += pos_diff;

    PARAMVALUE val;
    val.value = pos_diff;
    val.status = next -> status_lat;
    x_data.dist_x.push_back(val);

    return true;
}

/// Перебор возможных вариантов разрывов и их обработка
bool ScrTrack::enumPossible(vector<GEOPOINT>::iterator cur, vector<GEOPOINT>::iterator next)
{
    ScrTrack::GAPTYPE gapType = checkGaps();
    bool isContinue = false;

    switch(gapType)
    {
    case ScrTrack::GT_NONE:
        readed_time = 10;
    break;
    case ScrTrack::GT_TIMEGAP:
        isContinue = addTimeValue(cur, next);
    break;
    case ScrTrack::GT_DISTGAP:
        isContinue = addDistValue(cur, next);
    break;
    case ScrTrack::GT_TMDSTGAP:
        isContinue = addTimeValue(cur, next);
        isContinue = addDistValue(cur, next);
    break;
    }

    return isContinue;
}



Сообщение отредактировал AD - 11.8.2008, 15:06
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Red Devil
  опции профиля:
сообщение 12.8.2008, 10:12
Сообщение #23


Студент
*

Группа: Участник
Сообщений: 68
Регистрация: 6.6.2008
Из: Saint-Petersburg
Пользователь №: 194

Спасибо сказали: 1 раз(а)




Репутация:   3  


Код
class ScrTrack: public TImageClass
{
private:
    std::string m_sIniFileaame;
    int         nReadTime;          
    int         nReadDist;          
    bool         m_IsEventPicture;       
    bool         m_IsEventNamePress;    

protected:
    typedef std::vector <GEOPOINT>    GeoPointArray;
    GeoPointArray * m_pGeo;

public:
    enum GAPTYPE { GT_NONE = 0, GT_TIMEGAP, GT_DISTGAP, GT_TMDSTGAP };
    
    SubSahara * m_pSahara;
    bool         m_IsInitGaps;

public:
    ScrTrack(SubSahara* pSah): m_nReadTime(0), m_nReadDist(0), m_pGeo(0), m_pSahara(pSah), m_IsInitGaps(false),
                    bmpSymbol(NULL), m_IsEventNamePress(false), m_IsEventPicture(false) {}
    virtual ~ScrTrack() {}
    virtual void DrawClass(GRAPHIC_DEVICE& device, const CHART_SCOPE& scope, const DRAW_MODE& mode);
    virtual void LoadSymbols();
    virtual void UnloadSymbols();
    
    bool enumPossible(GeoPointArray::iterator cur, GeoPointArray::iterator next);
    bool GapNone();
    bool GapTime(GeoPointArray::iterator cur, GeoPointArray::iterator next);
    bool GapDist(GeoPointArray::iterator cur, GeoPointArray::iterator next);
    bool GapTDM(GeoPointArray::iterator cur, GeoPointArray::iterator next);
    
    bool checkStatusDist(GeoPointArray::iterator cur, GeoPointArray::iterator next);
    bool checkStatusBreak(GeoPointArray::iterator cur, GeoPointArray::iterator next);
};

bool ScrTrack::enumPossible(GeoPointArray::iterator cur, GeoPointArray::iterator next)
{
    switch(checkGaps())
    {
        case ScrTrack::GT_NONE:
            return GapNone();
            
        case ScrTrack::GT_TIMEGAP:
            return GapTime(cur, next);
            
        case ScrTrack::GT_DISTGAP:
            return GapDist(cur, next);
            
        case ScrTrack::GT_TMDSTGAP:
            return GapTDM(cur, next);
    }

    return true;
}

bool ScrTrack::GapNone()
{
    m_nReadTime = 10;
    return true;
}

bool ScrTrack::checkStatusBreak(GeoPointArray::iterator cur, GeoPointArray::iterator next)
{
    return next -> status_lat == PS_NONE || next -> status_lon == PS_NONE || cur -> status_lat == PS_NONE || cur -> status_lon == PS_NONE;
}

bool ScrTrack::checkStatusDist(GeoPointArray::iterator cur, GeoPointArray::iterator next)
{
    return next -> status_lat != PS_FAIL && next -> status_lon != PS_FAIL && cur -> status_lat != PS_FAIL
            && cur -> status_lon != PS_FAIL && next -> status_lat != PS_NODATA && next -> status_lon != PS_NODATA
            && cur -> status_lat != PS_NODATA && cur -> status_lon != PS_NODATA;
}

bool ScrTrack::GapTime(GeoPointArray::iterator cur, GeoPointArray::iterator next)
{
    if(checkStatusBreak(cur, next))
        return true;

    time_t time_diff = static_cast<time_t>(fabs(double(next -> time_marker - cur -> time_marker)));
    if(time_diff > m_nReadTime) 
        return false;
    
    if(!m_IsInitGaps)
        return true;
        
    if(checkStatusDist(cur, next)
        q_calc.maxTime += time_diff;
        
    PARAMVALUE val;
    val.value     = (double)time_diff;                
    val.status     = next -> status_lat;
    x_data.time_x.push_back(val);
    
    return true;    
}

bool ScrTrack::GapDist(GeoPointArray::iterator cur, GeoPointArray::iterator next)
{
    if(checkStatusBreak(cur, next))
        return true;
    
    double pos_diff = diffDist(cur, next);
    if(pos_diff > m_nReadDist)
        return false;
    
    if(!m_IsInitGaps)
        return true;
        
    if(checkStatusDist(cur, next))
        q_calc.maxDist += pos_diff;

    PARAMVALUE val;
    val.value     = pos_diff;
    val.status     = next -> status_lat;
    x_data.dist_x.push_back(val);
    return true;
}

bool ScrTrack::GapTDM(GeoPointArray::iterator cur, GeoPointArray::iterator next)
{
    if(checkStatusBreak(cur, next))
        return true;
    
    time_t time_diff = (time_t)fabs(double(next -> time_marker - cur -> time_marker));
    double pos_diff = diffDist(cur, next);
    if(time_diff > m_nReadTime || pos_diff > m_nReadDist)
        return false;
        
    if (!m_IsInitGaps)
        return true;
        
    if(checkStatusDist(cur, next))
    {
                q_calc.maxDist += pos_diff;
                q_calc.maxTime += time_diff;
    }
    
    PARAMVALUE val;
    val.value = pos_diff;
    val.status = next -> status_lat;
    x_data.dist_x.push_back(val);
    val.value = (double)time_diff;
    x_data.time_x.push_back(val);
    
    return true;
}
Причина редактирования: обернул в тэг expand
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
rich
  опции профиля:
сообщение 13.12.2008, 0:03
Сообщение #24


Участник
**

Группа: Участник
Сообщений: 123
Регистрация: 1.3.2008
Пользователь №: 109

Спасибо сказали: 6 раз(а)




Репутация:   0  


Может кому поможет:
"Рефакторинг. Улучшение существующего кода" Мартин Фаулер.
заказ книг
ещё заказ книг
и ещё заказ книг
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
Admin
  опции профиля:
сообщение 13.12.2008, 0:52
Сообщение #25


Администратор
****

Группа: Администратор
Сообщений: 646
Регистрация: 9.10.2007
Из: crossplatform.ru
Пользователь №: 1

Спасибо сказали: 17 раз(а)




Репутация:   2  


rich, а ты темой не ошибся? Причем тут упрощение кода?
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
alex977
  опции профиля:
сообщение 13.12.2008, 11:28
Сообщение #26


Активный участник
***

Группа: Участник
Сообщений: 310
Регистрация: 19.6.2008
Из: Россия, МО, г.Мытищи
Пользователь №: 206

Спасибо сказали: 77 раз(а)




Репутация:   8  


Цитата(Admin @ 13.12.2008, 0:52) *
rich, а ты темой не ошибся? Причем тут упрощение кода?


ИМХО, имеет отношение:
http://ru.wikipedia.org/wiki/Рефакторинг
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение
rich
  опции профиля:
сообщение 14.12.2008, 18:41
Сообщение #27


Участник
**

Группа: Участник
Сообщений: 123
Регистрация: 1.3.2008
Пользователь №: 109

Спасибо сказали: 6 раз(а)




Репутация:   0  


Цитата(Admin @ 13.12.2008, 0:52) *
rich, а ты темой не ошибся? Причем тут упрощение кода?

ИМХО, не ошибся. :)

Сообщение отредактировал rich - 14.12.2008, 18:43
Перейти в начало страницы
 
Быстрая цитата+Цитировать сообщение

3 страниц V   1 2 3 >
Быстрый ответОтветить в данную темуНачать новую тему
Теги
Нет тегов для показа


1 чел. читают эту тему (гостей: 1, скрытых пользователей: 0)
Пользователей: 0


RSS Рейтинг@Mail.ru Текстовая версия Сейчас: 4.5.2025, 8:56