Shortening this comparison code the right way

[jsmithplus]

This code is inside a while loop and version 1 works, but I guess the code is too big and readability is bad. The array that needs to be ckecked is always going to be 6 elements.

I'm not sure what happens in version two...it means that it should move on to the next iteration of the for loop, not the outside while loop? What is a better way to shorten this, but still using the continue statement for the surrounding while loop?

//Version 1
//Check against previous values
if (comboOne == collectedValues[0] || comboOne == collectedValues[1] || comboOne == collectedValues[2] || comboOne == collectedValues[3] || comboOne == collectedValues[4] || comboOne == collectedValues[5] || comboTwo == collectedValues[0] || comboTwo == collectedValues[1] || comboTwo == collectedValues[2] || comboTwo == collectedValues[3] || comboTwo == collectedValues[4] || comboTwo == collectedValues[5])
   {
   continue;
   }

//Version 2
for (int i = 0; i < collectionLength; i++)
{

if (comboOne == collectedValues[i] || comboTwo == collectedValues[i])
   {
   continue;
   }
}

[dutch]

The continue is continuing the inner for loop, not the outer while. There are two ways to fix this. Use a flag (or alternatively test the value of the loop index) or use a goto.

// Version 1: using a flag

bool do_continue = false;

for (int i = 0; i < collectionLength; i++)
{
    if (comboOne == collectedValues[i] || comboTwo == collectedValues[i])
    {
        do_continue = true;
        break;
    }
}

if (do_continue)
    continue;



// Version 1a: it can be done without a flag

int i = 0;
for ( ; i < collectionLength; i++)
{
    if (comboOne == collectedValues[i] || comboTwo == collectedValues[i])
    {
        break;
    }
}

if (i < collectionLength)   // must have hit the break
    continue;



// Version 2: using a goto

for (int i = 0; i < collectionLength; i++)
{
    if (comboOne == collectedValues[i] || comboTwo == collectedValues[i])
    {
        goto SkipContinue;
    }
}

continue;

SkipContinue:
    ;

The control structure in version 2 is formalized in python with the "else" clause of the "for" statement.

[Repeater]

You could use std::count to count how many times the item you're looking for is in the array. http://www.cplusplus.com/reference/algorithm/count/

if (count(collectedValues, collectedValues + 6, comboOne) &&
    count(collectedValues, collectedValues + 6, comboTwo))
{
  // both found in the array - change && to || if you only care about finding one
}

[George P]

<algorithm> has several variations of container searches. Searches that work with regular arrays or C++ STL containers. std::find could help you, if all you want to know is if a value exists in the container.

http://www.cplusplus.com/reference/algorithm/
http://www.cplusplus.com/reference/algorithm/find/

Do the searches and then see if either search gives a valid, not at the end, iterator.

[jonnin]

is this really

do
{
code
} while (something and all the combo tests could go here?)


if so, a cleaned up version one using repeater's ideas seems like where I would take it (eg move his counts down into the loop condition and lose the continue thing). Continue is cool when you need it but when you do not need it, it is just that much more weirdness in a code block to try to unravel.

if you need the continue, then it has to go in the body.