2015-02-19 2 views
0

У меня есть функция, которая находит все несколько элементов в векторе. Если я отправлю в {1,2,3,4,5,1,2,3,3,7}, он возвращает {1,2,3}. Мой входной вектор имеет от 100 до 10000 элементов, но я ожидаю, что у вас будет только очень мало разных (!) Дубликатов; около 1-5%.Как прыгать в конец цикла for, но не оставлять его, с goto

Следовательно, я проверяю свой вектор дубликатов, если я уже идентифицировал элемент как повторяющийся несколько раз. Если это так, функция должна перейти к следующему элементу, если они есть. Для этого я использую goto.

Но мне нужно иметь команду после goto label. Иначе компилятор жалуется. Есть ли способ избежать этого и сохранить goto? Я знаю, что могу использовать какой-то другой метод, например. устанавливая bool соответственно и используя if(). Однако я считаю, что метод goto прямолинейный.

vector<int> findDublicates(vector<int> const& v) { 
    // e.g. {1,2,3,4,5,1,2,3,7} -> {1,2,3} 
    vector<int> dublicates; 
    for (auto it(v.begin()); it != v.end() - 1; 
     ++it) { // go through each element except the last 
     for (auto const& i : 
      dublicates) { // check if this is already a known dublicate 
      if (i == *it) 
       goto nextElement; // if so, goto the next element in v 
     } 
     for (auto it2(it + 1); it2 != v.end(); 
      ++it2) { // else compare it with the "not checked" elements in v 
      if (*it == *it2) { // if a dublicate is found, keep it 
       dublicates.emplace_back(*it); 
       break; // check the next element in v; could also use goto 
         // nextElement 
      } 
     } 
    nextElement: 
     cout << " "; // if I remove cout it won't compile: "expected 
        // primary-expression before '}' token" 
    } 
    return dublicates; 
} 
+3

Команда может быть просто точкой с запятой. Это оператор null, но он должен удовлетворять компилятору. –

+1

'nextElement:;' –

+0

Похож на использование _good old_ для циклов с традиционными частями состояния, а 'break;' statement будет составлять более чистый код C++. –

ответ

5

Вы должны иметь возможность использовать точку с запятой как no-op.

nextElement: 
    ; 
} 

Однако я не уверен, эффективен ли ваш метод поиска дубликатов. Возможно, вам лучше отсортировать массив, а затем повторить его один раз. Сортировка вектора объединяет все дубликаты. Тогда вам просто нужно проверить, совпадает ли текущий элемент с предыдущим элементом.

+0

спасибо. В другой версии я использую его для поиска равных объектов, для которых сортировка как-то затруднена, но просто сравнение не ... – dani

+0

Вы можете также использовать std :: set, а не вектор. Наборы автоматически сортируются и в этом случае более эффективны, как предлагает г-н Лалама. – Andy

0

Если удаление goto не будет убивать вас, попробуйте использовать булевы переменные хелпер:

for(auto it(v.begin()); it!=v.end()-1; ++it) { //go through each element except the last 
    bool found = false; 
    for(auto const &i : dublicates) { //check if this is already a known dublicate 
     if(i==*it) 
     { 
      found = true; 
      break; 
     } 
    } 

    if (!found) 
    { 
     for(auto it2(it+1); it2!=v.end(); ++it2) { //else compare it with the "not checked" elements in v 
      if(*it==*it2) { //if a dublicate is found, keep it 
       dublicates.emplace_back(*it); 
       break; //check the next element in v; could also use goto nextElement 
      } 
     } 
    } 
    cout<<" "; //if I remove cout it won't compile: "expected primary-expression before '}' token" 
} 

Использования goto считается плохим тоном в программировании. Найдите в Интернете «спагетти код goto».

+0

Я бы подумал, что моя версия будет более читаемой, чем эта. – dani

+0

bool found не лучше, чем goto (с цепочкой из нескольких фильтров это становится досадой) –

0

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

Рассмотрим:

bool isDublicate(int candidate, vector<int> const & dublicates) 
{ 
    for (auto const &i: dublicates) 
     if (i == candidate) return true; 
    return false; 
} 

vector<int> findDublicates(vector<int> const &v) { 
    //e.g. {1,2,3,4,5,1,2,3,7} -> {1,2,3} 
    vector<int> dublicates; 
    for(auto it(v.begin()); it!=v.end()-1; ++it) { //go through each element except the last 
     if (isDublicate(*it, dublicates)) 
      continue; 

     for(auto it2(it+1); it2!=v.end(); ++it2) { //else compare it with the "not checked" elements in v 
      if(*it==*it2) { //if a dublicate is found, keep it 
       dublicates.emplace_back(*it); 
       break; //check the next element in v; could also use goto nextElement 
      } 
     } 
    } 
    return dublicates; 
} 
+0

Приятная попытка - но неполная - вам следует избегать продолжения и перерыва –

+0

@ DieterLücking why? –

+0

@ DieterLücking: break выведет вас из функции преждевременно. Continue снова выталкивает вас на вершину цикла, как и предполагалось ранее. – Andy