Un algoritmo (ligeramente) más rápido también evita el goto

Borrar de un std::vector nunca es rápido y debe evitarse. Lo mismo se aplica para copiar un std::vector. Al evitar ambos, también evita el goto. Por ejemplo

size_t containsPairs(std::vector<Card> const &hand) // no copy of hand
{
    size_t num_pairs = 0;
    std::unordered_set<size_t> in_pair;

    for(size_t i=0; i!=hand.size(); ++i)
    {
        if(in_pair.count(i)) continue;
        auto c1 = hand[i];
        for(size_t j=i+1; j!=hand.size(); ++j)
        {
            if(in_pair.count(j)) continue;
            auto c2 = hand[j];
            if (c1.getFace() == c2.getFace())
            {
                ++num_pairs;
                in_pair.insert(i);
                in_pair.insert(j);
            }
        }
    }
    return num_pairs;
}

Para manos grandes, este algoritmo sigue siendo lento, ya que O(N^2). Más rápido sería ordenar, después de lo cual los pares deben ser tarjetas adyacentes, dando un algoritmo O(N logN).

Sin embargo, más rápido, O (N) , es usar un unordered_set no para las tarjetas en parejas, pero para todas las demás cartas:

size_t containsPairs(std::vector<Card> const &hand) // no copy of hand
{
    size_t num_pairs = 0;
    std::unordered_set<Card> not_in_pairs;
    for(auto card:hand)
    {
        auto match = not_in_pairs.find(card));
        if(match == not_in_pairs.end())
        {
            not_in_pairs.insert(card);
        }
        else
        {
            ++num_pairs;
            not_in_pairs.erase(match);
        }   
    }
    return num_pairs;
}

Para hand.size() suficientemente pequeño, esto puede no ser más rápido que el código anterior, dependiendo del sizeof(Card) y/o el costo de su constructor. Un enfoque similar es usar distribución como se sugiere en Respuesta de Eric Duminil :

size_t containsPairs(std::vector<Card> const &hand) // no copy of hand
{
    std::unordered_map<Card,size_t> slots;
    for(auto card:hand)
    {
        slots[card]++;
    }
    size_t num_pairs = 0;
    for(auto slot:slots)
    {
        num_pairs += slot.second >> 1;
    }
    return num_pairs;
}

Por supuesto, estos métodos se pueden implementar mucho más simplemente si Card se puede asignar trivialmente a un entero pequeño, cuando no se requiere hash.

Por diversión aquí hay dos maneras más, les presento un método un poco más eficiente sin interrupciones o goto. Luego presento un método menos eficiente que ordena primero.

Ambos métodos son fáciles de leer y entender.

Estas son realmente solo para mostrar alternativas a las otras respuestas. El primer método containsPairs que tengo requiere que los valores de la tarjeta estén en el rango de 0 a 13 y se romperá si eso no es cierto, pero es un poco más eficiente que cualquiera de los otros respuestas que he visto.

int containsPairs(const vector<int> &hand)
{
    int pairs{ 0 };
    std::vector<int> counts(14); //note requires 13 possible card values
    for (auto card : hand){
        if(++counts[card] == 2){
            ++pairs;
            counts[card] = 0;
        }
    }
    return pairs;
}

int containsPairs(const vector<int> &hand)
{
    int pairs{ 0 };

    std::sort(hand.begin(), hand.end());
    for (size_t i = 1;i < hand.size();++i){
        if(hand[i] == hand[i - 1]){
            ++i;
            ++pairs;
        }
    }
    return pairs;
}

Nota: varias de las otras respuestas tratarán 3 cartas similares en una mano como 2 pares. Los dos métodos anteriores tienen esto en cuenta y en su lugar solo contarán 1 par para 3 de un tipo. Lo tratarán como 2 pares si hay 4 cartas similares.

goto es sólo un problema. Otro gran problema es que su método es ineficiente.

Su método

Su método actual básicamente mira la primera tarjeta, itera sobre el resto y busca el mismo valor. Luego vuelve a la segunda carta y la compara con el resto. Esto es O(n**2).

Ordenando

¿Cómo contarías los pares en la vida real? Probablemente ordenarías las cartas por valor y buscarías parejas. Si usted ordena eficientemente, sería O(n*log n).

Distribuyendo

El método más rápido sería preparar 13 ranuras en una mesa y distribuir las cartas de acuerdo con su valor nominal. Después de distribuir cada carta, puedes contar las cartas en cada ranura y ver si alguna ranura tiene al menos 2 cartas. Es O(n) y también detectaría tres de un tipo o cuatro de un tipo.

Claro, no hay mucha diferencia entre n**2 y n cuando n es 5. Como bono, el último método sería conciso, fácil de escribir y goto gratis.

Si realmente desea evitar goto, entonces puede llamar a la función recursivamente, donde estaría la línea goto [label], pasando cualquier variable cuyo estado desea guardar como parámetros. Sin embargo, yo recomendaría apegarse al goto.

Personalmente pondría esos dos bucles en una lambda, en lugar de goto volvería de esta lambda con indicación de que los bucles deberían reiniciarse, y llamaría a la lambda en un bucle. Algo así:

auto iterate = [&hand, &pairs]() {
             {
              ... // your two loops go here, instead of goto return true
             }
             return false;
}

while (iterate());

Small addition : No creo que este sea el mejor algoritmo para encontrar pares de cartas en un mazo. Hay opciones mucho mejores para eso. Prefiero responder a la pregunta omnipresente de cómo transferir el control dentro o fuera de dos bucles a la vez.

Sí, debe evitar usar goto aquí.

Es un uso innecesario de goto específicamente porque el algoritmo no lo necesita. Como un aparte, tiendo a no usar goto, pero no estoy en firme oposición a ella como muchos. goto es una gran herramienta para romper bucles anidados o para salir de una función limpiamente cuando una interfaz no soporta RAII.

Hay algunas ineficiencias con su enfoque actual:

• No hay razón para volver a buscar en la lista desde el principio cuando encuentre un par que coincida. Ya ha buscado todas las combinaciones anteriores. La eliminación de cartas no cambia el orden relativo de las cartas no eliminadas y, además, no le proporciona más pares.
• No hay necesidad de eliminar elementos de la mitad de hand. Para este problema eliminar del medio de un std::vector presumiblemente representando una mano de 5 cartas no es un problema. Sin embargo, si el número de tarjetas es grande, esto puede ser ineficiente. En problemas así que deberías preguntarte, ¿importa el orden de los elementos? La respuesta es no, no importa. Podemos barajar cualquier carta que no haya sido emparejada y aún así lograr la misma respuesta.

Aquí está una versión modificada de su código:

int countPairs(std::vector<Card> hand)
{
    int pairs{ 0 };

    for (decltype(hand.size()) i = 0; i < hand.size(); ++i)
    {
        // I assume getFace() has no side-effects and is a const
        // method of Card.  If getFace() does have side-effects
        // then this whole answer is flawed.
        const Card& c1 = hand[i];
        for (auto j = i + 1; j < hand.size(); ++j)
        {
            const Card& c2 = hand[j];
            if (c1.getFace() == c2.getFace())
            {
                // We found a matching card for card i however we
                // do not need to remove card i since we are
                // searching forward.  Swap the matching card
                // (card j) with the last card and pop it from the
                // back.  Even if card j is the last card, this
                // approach works fine.  Finally, break so we can
                // move on to the next card.
                pairs++;
                std::swap(c2, hand.back());
                hand.pop_back(); // Alternatively decrement a size variable
                break;
            }
        }
    }
    return pairs;
}

Puede retocar el enfoque anterior para usar iteradores si lo desea. También puede tomar una referencia const std::vector y usar std::reference_wrapper para volver a ordenar el contenedor.

Para una mejor compilación del algoritmo en general una tabla de frecuencia de cada valor facial y su recuento correspondiente.

Probablemente lo haría de esta manera:

Características:

• 3 de un tipo no es un par
• devuelve un vector de cartas en orden descendente indicando qué caras son pares en la mano.

std::vector<Card> reduceToPair(std::vector<Card> hand)
{
    auto betterFace = [](auto&& cardl, auto&& cardr)
    {
        return cardl.getFace() > cardr.getFace();
    };

    std::sort(begin(hand), end(hand), betterFace);

    auto first = begin(hand);
    while (first != end(hand))
    {
        auto differentFace = [&](auto&& card)
        {
            return card.getFace() != first->getFace();
        };
        auto next = std::find_if(first + 1, end(hand), differentFace);
        auto dist = std::distance(first, next);
        if (dist == 2)
        {
            first = hand.erase(first + 1, next);
        }
        else
        {
            first = hand.erase(first, next);
        }
    }

    return hand;
}

Uso:

pairInfo = reduceToPair(myhand);
bool hasPairs = pairInfo.size();
if (hasPairs)
{
  auto highFace = pairInfo[0].getFace();
  if (pairInfo.size() > 1) {
    auto lowFace = pairInfo[1].getFace();
  }
}

Si la clasificación de cartas por cara es posible y está permitida, podemos contar parejas usando una sola pasada sin borrar nada:

bool Compare_ByFace(Card const & left, Card const & right)
{
    return(left.Get_Face() < right.Get_Face());
}

size_t Count_Pairs(vector<Card> hand)
{
    size_t pairs_count{0};
    if(1 < hand.size())
    {
        sort(hand.begin(), hand.end(), &Compare_ByFace);
        auto p_card{hand.begin()};
        auto p_ref_card{p_card};
        for(;;)
        {
           ++p_card;
           if(hand.end() == p_card)
           {          
               pairs_count += static_cast< size_t >((p_card - p_ref_card) / 2);
               break;
           }
           if(p_ref_card->Get_Face() != p_card->Get_Face())
           {
               pairs_count += static_cast< size_t >((p_card - p_ref_card) / 2);
               p_ref_card = p_card;
           }
        }
    }
    return(pairs_count);
}

#include <vector>
#include <unordered_map>
#include <algorithm>

std::size_t containsPairs(const std::vector<int>& hand)
{
    // boilerplate for more readability
    using card_t = std::decay_t<decltype(hand)>::value_type;
    using map_t = std::unordered_map<card_t, std::size_t>;

    // populate map and count the entrys with 2 occurences
    map_t occurrences;
    for (auto&& c : hand) { ++occurrences[c]; }
    return std::count_if( std::cbegin(occurrences), std::cend(occurrences), [](const map_t::value_type& entry){ return entry.second == 2; });
}

Un problema con goto es que las etiquetas tienden a ser walkies en la refactorización errante. Eso es fundamentalmente por qué no me gustan. Personalmente, en su caso, si necesita mantener el algoritmo tal como está, me gustaría rodar el goto en una llamada recursiva:

int containsPairs(vector<Card>&/*Deliberate change to pass by reference*/hand)
{
    for (int i = 0; i < hand.size(); i++)
    {
        Card c1 = hand[i];
        for (int j = i + 1; j < hand.size(); j++)
        {
            Card c2 = hand[j];
            if (c1.getFace() == c2.getFace())
            {
                hand.erase(hand.begin() + i);
                hand.erase(hand.begin() + (j - 1));
                return 1 + containsPairs(hand); 
            }
        }
    }
    return 0;
}

La sobrecarga en la creación del marco de pila es insignificante cf. las std::vector manipulaciones. Esto podría ser poco práctico dependiendo del sitio de la llamada: ya no puede llamar a la función con un temporal anónimo, por ejemplo. Pero realmente hay mejores alternativas para la identificación de pares: ¿por qué no ordenar la mano de manera más óptima?

¿Se le permite cambiar el orden de los elementos en un vector? En caso afirmativo, utilice un algoritmo adjacent_find en un solo bucle.

Así no solo te desharás de goto, sino que obtendrás un mejor rendimiento (actualmente tienes O(N^2)) y una corrección garantizada:

std::sort(hand.begin(), hand.end(), 
    [](const auto &p1, const auto &p2) { return p1.getFace() < p2.getFace(); });
for (auto begin = hand.begin(); begin != hand.end(); )
{
  begin = std::adjacent_find(begin, hand.end(), 
        [](const auto &p1, const auto &p2) { return p1.getFace() == p2.getFace(); });
  if (begin != hand.end())
  {
    auto distance = std::distance(hand.begin(), begin);
    std::erase(begin, begin + 2);  // If more than 2 card may be found, use find to find to find the end of a range
    begin = hand.begin() + distance;
  }
}

Su implementación no está funcionando, ya que cuenta tres de un tipo como un par, cuatro de un tipo como dos.

Aquí hay una implementación que sugeriría:

int containsPairs(std::vector<Card> hand)
{
    std::array<int, 14> face_count = {0};
    for (const auto& card : hand) {
        ++face_count[card.getFace()]; // the Face type must be implicitly convertible to an integral. You might need to provide this conversion or use an std::map instead of std::array.
    }
    return std::count(begin(face_count), end(face_count), 2);
}

(demo sobre coliru )

Se puede generalizar para contar no solo pares, sino también n de una especie ajustando el 2.

Las otras respuestas hasta ahora se refieren a cómo reestructurar fundamentalmente su código. Hacen el punto de que su código no era muy eficiente para empezar, y para el momento en que arregló que solo necesita salir de un bucle, por lo que no necesita el goto de todos modos.

Pero voy a responder a la pregunta de cómo evitar goto sin cambiar fundamentalmente el algoritmo. La respuesta (como es muy a menudo el caso para evitar goto) es mover parte de su código en una función separada y use un return temprano:

void containsPairsImpl(vector<Card>& hand, int& pairs)
{
    for (int i = 0; i < hand.size(); i++)
    {
        Card c1 = hand[i];
        for (int j = i + 1; j < hand.size(); j++)
        {
            Card c2 = hand[j];
            if (c1.getFace() == c2.getFace())
            {
                pairs++;
                hand.erase(hand.begin() + i);
                hand.erase(hand.begin() + (j - 1));
                return;
            }
        }
    }
    hand.clear();
}

int containsPairs(vector<Card> hand)
{
    int pairs{ 0 };
    while (!hand.empty()) {
        containsPairsImpl(hand, pairs);
    }
    return pairs;
}

Observe que paso hand y pairs por referencia a la función interna para que puedan actualizarse. Si tiene muchas de estas variables locales, o tiene que romper la función en varias piezas, entonces esto puede volverse difícil de manejar. La solución entonces es usar una clase:

class ContainsPairsTester {
public:
    ContainsPairsTester(): m_hand{}, m_pairs{0} {}

    void computePairs(vector<Card> hand);

    int pairs() const { return m_pairs; }
private:
    vector<Card> m_hand;
    int m_pairs;

    void computePairsImpl(vector<Card> hand);
};

void ContainsPairsTester::computePairsImpl()
{
    for (int i = 0; i < m_hand.size(); i++)
    {
        Card c1 = m_hand[i];
        for (int j = i + 1; j < m_hand.size(); j++)
        {
            Card c2 = m_hand[j];
            if (c1.getFace() == c2.getFace())
            {
                m_pairs++;
                m_hand.erase(m_hand.begin() + i);
                m_hand.erase(m_hand.begin() + (j - 1));
                return;
            }
        }
    }
    m_hand.clear();
}

void ContainsPairsTester::computePairs(vector<Card> hand)
{
    m_hand = hand;
    while (!m_hand.empty()) {
        computePairsImpl();
    }
}

Como otros han comentado, no solo debe evitar goto, sino también evitar escribir su propio código donde hay un algoritmo estándar que puede hacer el trabajo. Me sorprende que nadie haya sugerido único, que está diseñado para este propósito:

bool cardCmp(const Card& a, const Card& b) {
    return a.getFace() < b.getFace();
}

size_t containsPairs(vector<Card> hand) {
    size_t init_size = hand.size();

    std::sort(hand.begin(), hand.end(), cardCmp);
    auto it = std::unique(hand.begin(), hand.end(), cardCmp);
    hand.erase(it, hand.end());

    size_t final_size = hand.size();
    return init_size - final_size;
}

(Primera respuesta en StackOverflow-disculpas por cualquier paso en falso!)

Mientras que goto no es realmente tan horrible si lo necesitas, no es necesario aquí. Dado que solo te importa el número de pares, tampoco es necesario registrar cuáles son esos pares. Puede simplemente xor a través de toda la lista.

Si está utilizando GCC o clang, lo siguiente funcionará. En MSVC, puede usar __popcnt64() en su lugar.

int containsPairs(vector<Card> hand)
{
    size_t counter = 0;
    for ( Card const& card : hand )
        counter ^= 1ul << (unsigned) card.getFace();

    return ( hand.size() - __builtin_popcountll(counter) ) / 2u;
}