Упрощение кода, написание того или иного кода более понятно и просто Опции

[AD]

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

Вот ее код:

/// Класс экранных траекторий
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]

Вынеси длинные проверки с next -> status... в отдельные функции - сразу жить станет легче, и станит видно что делать дальше.

[AD]

Вынеси длинные проверки с 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]

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

По коду:

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]

Tonal, я не понял как и что. Можно все-таки кодом, пожалуйста!

[AD]

А мне помогут упростить код?

[Tonal]

Чё непонятно-то?

[AD]

Чё непонятно-то?

непонятно что и как надо сделать!

[Tonal]

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

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

Да, и функции, вместо checkStatus* я бы назвал isStatus* - во первых сразу понятно что они возвращают и зачем, во вторых соответствует стилю Qt.

[AD]

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

Что сделать надо, что именно и куда вынести?