повысить проблему лямбда или феникс: использование std :: for_each для работы с каждым элементом контейнера - PullRequest
2 голосов
/ 22 октября 2008

Я столкнулся с проблемой при очистке старого кода. Это функция:

uint32_t ADT::get_connectivity_data( std::vector< std::vector<uint8_t> > &output )
{
    output.resize(chunks.size());
    for(chunk_vec_t::iterator it = chunks.begin(); it < chunks.end(); ++it)
    {
        uint32_t success = (*it)->get_connectivity_data(output[it-chunks.begin()]);
    }
    return TRUE;
}

Что мне интересно, так это очистить цикл for, чтобы он стал лямбда-выражением, но быстро застрял на том, как именно я передам правильный аргумент get_connectivity_data. get_connectivity_data берет std :: vector по ссылке и заполняет его некоторыми данными. вывод содержит std :: vector для каждого «чанка».

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

EDIT:

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

 std::for_each( chunks.begin(), chunks.end(), 
                   bind( &chunk_vec_t::value::type::get_connectivity_data, 
                         _1, 
                         output[ std::distance( _1, chunks.begn() ] 
                       )
                 );

Тем не менее, этот код не компилируется, я внес некоторые изменения в код, чтобы заставить его компилироваться, но столкнулся с двумя проблемами:

  1. _ 1 - это умный ptr, и std :: distance не сработал, думаю, мне нужно было использовать & chunks [0] в качестве начала
  2. Поскольку _ 1 - это умный указатель, мне пришлось сделать: & chunk_vec_t :: value_ type :: ValueType :: get_ connectivity_ data, которая вызвала сбой в компиляторе VC9 ...

Ответ, касающийся итераторов zip_, выглядел неплохо, пока я не прочитал больше об этом и не обнаружил, что для этого конкретного использования количество дополнительного кода было существенным (связывание того и этого и т. Д.).

EDIT2:

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

std::transform(chunks.begin(), chunks.end(), back_inserter(tmp), boost::bind(&ADTChunk::get_connectivity_data, _1) );

Ответы [ 6 ]

3 голосов
/ 23 октября 2008

После небольшой работы я придумал это решение:

std::transform(chunks.begin(), chunks.end(), back_inserter(tmp), boost::bind(&ADTChunk::get_connectivity_data, _1) );

Требуется, чтобы я изменил get_connectivity_data, чтобы он возвращал std :: vector вместо того, чтобы брать один по ссылке, и также потребовал, чтобы я изменил элементы чанков на boost :: shared_ptr вместо Loki :: SmartPtr.

2 голосов
/ 22 октября 2008

Не видя код для всего класса, трудно определить, что будет работать. Лично я думаю, что BOOST_FOREACH в этом случае чище, но для справки я мог бы попытаться сделать что-то подобное, используя лямбды (заметьте, я не могу проверить компиляцию)

uint32_t ADT::get_connectivity_data( std::vector< std::vector<uint8_t> > &output )
{
    using namespace boost::lambda;

    output.resize( chunks.size() );

    std::for_each( chunks.begin(), chunks.end(), 
                   bind( &chunk_vec_t::value::type::get_connectivity_data, 
                         _1, 
                         output[ std::distance( _1, chunks.begn() ] 
                       )
                 );
    return TRUE;
}
2 голосов
/ 22 октября 2008

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

1 голос
/ 22 октября 2008

По какой-то причине новички в STL всегда настаивают на использовании вектора :: iterator вместо более читабельного (и с постоянным временем) оператора []. Выражение it-chunks.begin() должно было сказать первоначальному автору, что он уже проиграл умную игру-кодер, и в конце концов ему нужен скромный индекс:

for (size_t i = 0, size = chunks.size(); i < size; ++i)
{
    chunks[i]->get_connectivity_data(output[i]);
} 

OP может также рассмотреть возможность потери ложного кода возврата и сделать его функцией-константой.

1 голос
/ 22 октября 2008

На самом деле вы выполняете операцию над двумя контейнерами параллельно. Вот для чего предназначен boost :: zip_iterator.

Однако единственная причина, по которой вам нужно обрабатывать контейнеры параллельно, это то, что Chunk :: get_connectivity_data принимает параметр out. Если бы он возвращался по значению (используя исключения для сообщения об ошибках), вы могли бы просто использовать итератор вставки.

1 голос
/ 22 октября 2008

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

Использовать typedefs для всех типов контейнеров STL:

typedef std::vector<uint8_t> Chunks;
typedef std::vector<Chunks> Output;
uint32_t ADT::get_connectivity_data( Output &output )

Поскольку вы говорите об использовании Boost, используйте Boost.Foreach :

BOOST_FOREACH(chunk_vec_t::value_type &chunk, chunks)
  uint32_t success =
    chunk->get_connectivity_data(output[std::distance(&chunk, chunks.begin())]);

Ударьте в темноте "лямбду":

typedef const boost::function2<uint32_t, chunk_vec_t::value_type, Chunks>
  GetConnectivity;
uint32_t ADT::get_connectivity_data(Output &output, GetConnectivity &getConnectivity)
{
  output.resize(chunks.size());
  BOOST_FOREACH(chunk_vec_t::value_type &chunk, chunks)
    uint32_t success =
      getConnectivity(chunk, output[std::distance(&chunk, chunks.begin())]);
  return TRUE;
}

Тогда вы можете назвать это как:

get_connectivity_data(output,
  boost::bind(&chunk_vec_t::value_type::get_connectivity_data, _1, _2));
...