Форум программистов [Powered by Invision Power Board]

Версия для печати темы
Нажмите сюда для просмотра этой темы в оригинальном формате

Форум программистов > C/C++: Общие вопросы > Качество кода: вектор векторов и т.д.

Автор: nerezus 13.5.2009, 01:52

Код


vector< vector<string> > regex_search_all(string text, string pattern) {
    vector< vector<string> > results;
    string::const_iterator start, end;
    boost::regex re(pattern);
    boost::match_results<string::const_iterator> res;
    start = text.begin();
    end = text.end();
    boost::match_flag_type flags = boost::match_default;
    while(boost::regex_search(start, end, res, re, flags)) {
        vector<string> line;
        for(int i = 0; i < res.size(); i++) {
            line.push_back(string(res[i].first, res[i].second));
        }
        results.push_back(line);
        flags |= boost::match_prev_avail;
        flags |= boost::match_not_bob;
        start = res[0].second;
    }
    return results;
}

Я не умею писать на C++. Это видно из качества данного кода.
Кто хочет мне дать какие советы?

Автор: Lazin 13.5.2009, 08:28

что-бы не возвращать вектор из ф-ии и избежать лишнего копирования, можно передавать в нее insert iterator, тип можно не указывать явно

Код


template<class InsertIterator>
void regex_search_all(string text, string pattern, InsertIterator i) 
{
...
    //results.push_back(line);
    *i++ = line;
...
}

использовать можно так:

Код


typedef std::vector< std::vector< std::string > > search_results_type;
search_results_type results;
std::insert_iterator< search_results_type > inserter(results, results.end());
regex_search_all(text, pattern, inserter);

Автор: azesmcar 13.5.2009, 08:38

nerezus

1.vector< vector<string> > - не мешало бы тайпдефнуть это безобразие smile

2. возвращать vector по ссылке, или как сказал Lazin
3.

Цитата(nerezus @ 13.5.2009, 01:52

)

string::const_iterator start, end;
boost::regex re(pattern);
boost::match_results<string::const_iterator> res;
start = text.begin();
end = text.end();

почему бы не инициализировать start и end сразу там где они нужны?

Код


    string::const_iterator start = text.begin();
    string::const_iterator end = text.end();

а вообще зачем нужны эти переменные?

Цитата

for(int i = 0; i < res.size(); i++) {
line.push_back(string(res[i].first, res[i].second));
}

этот цикл наверняка можно заменить на алгоритм std::copy (я просто точно не знаю что там буст возвращает)

Добавлено через 6 минут и 6 секунд

Цитата

flags |= boost::match_prev_avail;
flags |= boost::match_not_bob;

зачем это нужно в цикле?

Автор: xvr 13.5.2009, 14:06

Здесь явно напрашивается класс

Код


class RegexSerachAll {
 vector<pair<size_t,size_t> > results;
 vector<string> lines;

public:
 RegexSerachAll (string text, string pattern) {
    string::const_iterator start, end;
    boost::regex re(pattern);
    boost::match_results<string::const_iterator> res;
    start = text.begin();
    end = text.end();
    boost::match_flag_type flags = boost::match_default;
    while(boost::regex_search(start, end, res, re, flags)) {
        size_t mark=lines.size();
        for(int i = 0; i < res.size(); i++) {
            lines.push_back(string(res[i].first, res[i].second));
        }
        results.push_back(make_pair(mark,lines.size());
        flags |= boost::match_prev_avail;
        flags |= boost::match_not_bob;
        start = res[0].second;
    }
};

Двойной итератор по results->lines оставляю в качестве упражнения smile

Автор: nerezus 13.5.2009, 14:29

Цитата

что-бы не возвращать вектор из ф-ии и избежать лишнего копирования, можно передавать в нее insert iterator

Чем это лучше передачи results по ссылке?

Цитата

typedef std::vector< std::vector< std::string > > search_results_type;

Цитата

1.vector< vector<string> > - не мешало бы тайпдефнуть это безобразие

done

Цитата

2. возвращать vector по ссылке, или как сказал Lazin

Что лучше: итератор, ссылка, или vector< vector< string > > * ?

Цитата

этот цикл наверняка можно заменить на алгоритм std::copy (я просто точно не знаю что там буст возвращает)

Цитата

flags |= boost::match_prev_avail;
flags |= boost::match_not_bob;

Насколько я понял, это так надо для работы данной либы для такого поиска.

Цитата

Здесь явно напрашивается класс

Не думаю. Обычная функция с 3 переменными(2 in, 1 out). Даже обернутая в класс не добавит удобаства.

Автор: azesmcar 13.5.2009, 14:34

Цитата(nerezus @ 13.5.2009, 14:29

)

Чем это лучше передачи results по ссылке?

STL-овский стиль.

Цитата(nerezus @ 13.5.2009, 14:29

)

Насколько я понял, это так надо для работы данной либы для такого поиска.

да, но зачем в цикле? значение то от этого не меняется.

если
boost::match_results<string::const_iterator> res;
стл совместимый контейнер, то цикл можно заменить на std::copy.

Автор: Lazin 13.5.2009, 14:35

Цитата(nerezus @ 13.5.2009, 14:29

)

Чем это лучше передачи results по ссылке?

гибкостью, можно передавать любой итератор, у которого value_type = std::vector< std::string >

Автор: xvr 13.5.2009, 19:37

Цитата(nerezus @ 13.5.2009, 14:29)

Цитата

Здесь явно напрашивается класс

Не думаю. Обычная функция с 3 переменными(2 in, 1 out). Даже обернутая в класс не добавит удобаства.

Позволит избавится от вектора векторов, довольно расточительная конструкция smile

Автор: nerezus 15.5.2009, 12:09

Цитата

можно передавать любой итератор, у которого

По смыслу этого не надо делать. Внешняя структур всегда будет вектором.

Цитата

Позволит избавится от вектора векторов, довольно расточительная конструкция

Почему расточительная? Только за счет генерации?
А если я хочу написать что-то многомерное, есть ли приемлемый контейнер для этого?

P.S. Какой контейнер может держать разнотипные данные? Или, как я понял, это зло, и надо использоватьь структуры/классы?

Автор: zim22 15.5.2009, 12:18

Цитата(nerezus @ 15.5.2009, 12:09

)

P.S. Какой контейнер может держать разнотипные данные?

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

Автор: xvr 15.5.2009, 13:46

Цитата(nerezus @ 15.5.2009, 12:09)

Цитата

Позволит избавится от вектора векторов, довольно расточительная конструкция

Почему расточительная? Только за счет генерации?

За счет увеличения количества векторов. Если во внешнем векторе будет много маленьких внутренних векторов, то такая конструкция приведет к массе алокаций маленьких кусочков памяти вместо одного большого (как будет в случае 2х векторов - вектор всех внутренних элементов и вектор индексов).
Это ОЧЕНЬ неэффективно по скорости, кроме того, это может привести к фрагментации кучи.

Добавлено через 44 секунды

Цитата(nerezus @ 15.5.2009, 12:09)

P.S. Какой контейнер может держать разнотипные данные?

boost::any

Автор: nerezus 15.5.2009, 14:29

Цитата

кроме того, это может привести к фрагментации кучи.

Я думал, что память под него выделяется в стеке.

Автор: mes 15.5.2009, 14:51

Цитата(nerezus @ 15.5.2009, 13:29

)

Я думал, что память под него выделяется в стеке.

не путайте "под него" и "под его данные". Вектор - это оболочка над динамическим массивом,
и при стандартной аллокации, память под массив выделяется в куче.

Автор: Lazin 15.5.2009, 14:51

Цитата(nerezus @ 15.5.2009, 14:29

)

Я думал, что память под него выделяется в стеке.

под сам вектор - да, под данные, которые он хранит - нет, это-же динамический массив