2016-12-22 5 views
7

Пытается написать метод, который удаляет первые (с наименьшими ключами) N элементов из std :: map. Пробовал это:Удалить первые N элементов с std :: map?

void EraseNMapElements(const int numElementsToRemove){ 

    const int originalSize = _map.size();  
    auto eraseIter = _map.begin(); 
    std::advance(eraseIter, numElementsToRemove); 

    _map.erase(_map.begin(), eraseIter); 

    assert(_map.size() == (originalSize - numElementsToRemove)) || (0 == originalSize) || (0 == _map.size())); 
} 

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

Есть ли опрятный способ покрыть это? Я мог бы вытолкнуть оператор IF вокруг проверки на numElementsToRemove больше, чем map.size(), но должно быть лучшее решение?

+4

Почему было бы лучшее решение, чем самое простое решение? –

ответ

4

std::advance(i, n) имеет предварительное условие, что i может быть увеличен не менее чем на n. В вашем коде вы не проверяете это предварительное условие, поэтому, если вы вызываете его с помощью numElementsToRemove > originalSize, вы нарушаете это предварительное условие и, таким образом, испытываете неопределенное поведение. Чтобы исправить это, необходимо выполнить проверку перед вызовом std::advance, возможно, с помощью std::min:

auto realNumToRemove = std::min(numElementsToRemove, originalSize); 
std::advance(eraseIter, realNumToRemove); 
4

if заявление обеспечивает простое, читаемый решение:

if (originalSize <= numElementsToRemove) { 
    auto eraseIter = _map.begin(); 
    std::advance(eraseIter, numElementsToRemove); 

    _map.erase(_map.begin(), eraseIter); 
} else { 
    _map.clear(); // or whatever's appropriate 
} 
1

Самый простой способ, я могу видеть, чтобы сделать это было бы использовать std::next и оператор if.

void EraseNMapElements(const int numElementsToRemove) 
{ 
    if (_map.size() < numElementsToRemove) 
     _map.erase(_map.begin(), std::next(_map.begin(), numElementsToRemove)); 
    else 
     _map.clear(); 
} 

Обратите внимание, что хотя _map.size() < numElementsToRemove имеет знак/без знака несоответствие. Предпочтительно numElementsToRemove должен быть std::size_t или decltype(_map)::size_type.

2

Одна вещь, о которой еще не упоминалось, и это хорошо знать, так как C++ 11 std :: map :: erase (const_iterator) фактически возвращает итератор в следующий элемент. Таким образом, код может быть записан как:

auto i = _map.begin(); 
while (i != _map.end() && numElementsToRemove > 0) 
{ 
    i = _map.erase(i); 
    --numElementsToRemove; 
} 

Это будет перемещаться по элементам, чтобы стереть один раз, а не дважды.

+2

Правда. В то же время вполне возможно, что многоэлементная «стирание» может быть оптимизирована для того, чтобы резко сократить количество необходимых операций балансировки по сравнению с стиранием раз в раз. Как всегда: при рассмотрении performace, измеряйте. – Angew