-1

What can I do to silence this warning? Do I need to add another return statement somewhere or do I need to change something within the functions?

Also could someone help me add arrows into the Johnson-Trotter algorithm. It would be nice to have them to show the direction but I am very confused on how to do it; though this isn't the main concern right now I just want the program to run. Thank you in advance.

These are the two functions with the warning:


   int searchArr(int k[], int n, int mobile)
   {
       for(int i = 0; i < n; i++)
       {
         if (k[i] == mobile)
         {
            return i + 1;
         }
       }
   }


int printOnePerm(int k[], bool dir[], int n)
{
    int mobile = getMobile(k, dir, n);

    int pos = searchArr(k, n, mobile);

    if (dir[k[pos - 1] - 1] == RIGHT_TO_LEFT)
    {
        swap(k[pos - 1], k[pos -2]);
    }
    else if (dir[k[pos - 1] - 1] == LEFT_TO_RIGHT)
    {
        swap(k[pos], k[pos -1]);
    }

    for(int i = 0; i < n; i++)
    {
        if (k[i] > mobile)
        {
            if (dir[k[i] - 1] == LEFT_TO_RIGHT)
            {
                dir[k[i] - 1] = RIGHT_TO_LEFT;
            }
            else if(dir[k[i] - 1] == RIGHT_TO_LEFT)
            {
                dir[k[i] - 1] = LEFT_TO_RIGHT;
            }
        }
    }

    for(int i = 0; i < n; i++)
    {
        cout << k[i];
    }

    cout << endl;
}
Xander
  • 3
  • 1
  • 3
    If searchArr() does not find a match what does it return? What is the int value that printOnePerm() should return - since your code says it has to return one ? – Avi Berger Nov 13 '19 at 23:04

2 Answers2

1

For the first function, searchArr(), one question is what do you expect it to return if the value is not found. Since the return values are in the range [1,n], I'm guessing that zero means not found.

I prefer to design functions which have a single return at the end, whenever possible. A default fail value can be set at the start of the function. I would exit the loop when the value is found, or fall through with the default value set.

Here is what I would write:

int searchArr(int k[], int n, int mobile)
   {
       int ret = 0;  /* not found value */

       for(int i = 0; i < n; i++)
       {
         if (k[i] == mobile)
         {
            ret = i + 1;
            break;
         }
       }

       return ret;
   }

Alternately, and perhaps a bit more obscurely, if the value is not found in the array, then i will equal n when the for loop completes. This would be a possible function:

int searchArr(int k[], int n, int mobile)
   {
       for(int i = 0; i < n; i++)
       {
         if (k[i] == mobile)
         {
            break;
         }
       }

       if (i < n)
          return i + 1;
       else
          return 0;
   }

The for loop can be shrunk to

for(int i = 0; i < n && k[i] != mobile; i++) ;

And the return can be shrunk to

return (i < n) ? i + 1 : 0;

Although I generally discourage using the ?: operator.

As mentioned above, the second function doesn't return any value and should be declared "void".

Mike Eager
  • 309
  • 1
  • 2
  • 9
0

The first one:

int searchArr(int k[], int n, int mobile)
   {
       for(int i = 0; i < n; i++)
       {
         if (k[i] == mobile)
         {
            return i + 1;
         }
       }
   }

will not return anything if for some reason nothing in your array matches. In that case, you need to return a default or error value:

int searchArr(int k[], int n, int mobile)
   {
       for(int i = 0; i < n; i++)
       {
         if (k[i] == mobile)
         {
            return i + 1;
         }
       }
   return -1; // not found
   }

The second one doesn't seem to want to return anything. In C++, the way to do this is with a void, not an int (That was okay in C. C++ not so much):

// assuming we don't want to return anything
void printOnePerm(int k[], bool dir[], int n)