Cycle through stored names and skip blanks

Question

I am trying to make my code more efficient. On scroll button push, the function 'SetName' stores a number to integer iName which is index against 5 names stored in memory. If a name is not set in memory, it skips to the next.

The code works, but takes up a lot of room. Any advice is appreciated. Code:

    #include <string.h>
    int iName = 0;
    int iNewName = 0;

    BYTE GetName ()
    {
     return  iName;
    }


    void SetName (int iNewName)
    {

      while (iName != iNewName)
      {

      switch (byNewName)
       {
        case 1:
        if (strlen (memory.m_nameA) == 0)
         new_name++;
        else
         iName = iNewName;
        break;

        case 2:
        if (strlen (memory.m_nameB) == 0)
         new_name++;
        else
         iName = iNewName;
        break;

        case 3:
        if (strlen (memory.m_nameC) == 0)
         new_name++;
        else
         iName = iNewName;
        break;

        case 4:
        if (strlen (memory.m_nameD) == 0)
         new_name++;
        else
         iName = iNewName;
        break;

        case 5:
        if (strlen (memory.m_nameE) == 0)
         new_name++;
        else
         iName = iNewName;
        break;

        default:
        iNewName = 1;
        break;
       } // end of case

      } // end of loop

    } // end of SetName function


void main ()
 {
  while(1)
  {
  if (Button_pushed)
   SetName(GetName+1);
  } // end of infinite loop
} // end of main

Answer 1

Replace all the lines of the form:

if (strlen (memory.m_nameA) == 0)

with

if( *memory.m_nameA == '\0' )

That will remove the dependency on strlen() and string.h. Testing for an empty string is a special case that does not require you to determine the length of the string. If you are not using it elsewhere, it will be removed entirely from the code.

In fact you have unnecessary repeated code selected on an index. Use a look-up table instead:

void SetName (int iNewName)
{
    static const char* lookup[] = { memory.m_nameA, 
                                    memory.m_nameB,
                                    memory.m_nameC,
                                    memory.m_nameD,
                                    memory.m_nameE } ;


  while (iName != iNewName)
  {

      iNewName = 1;
      if( byNewName < sizeof(lookup) / sizeof(*lookup) )
      {
          if( *lookup[byNewName] == '\0' )
          {
              new_name++;
          }
          else
          {
              iName = iNewName;
          }
      }
   } // end of loop
}

If the names in memory were already an array, then you would not need a look-up table at all.

Answer 2

Could new_name be static?

#include <string.h>
static int iName = 0;
static int iNewName = 0;

BYTE GetName ()
{
 return  iName;
}

void shortCut(char[] m,int N) //may not work for other type of arrays because their pointers are diminished into a single pointer that cannot give "sizeof" value. But strlen function checks for a null element in string.
{
   if (strlen (m) == 0)
     new_name++;
   else
     iName = N;
}

void SetName (int iNewName)
{

  while (iName != iNewName)
  {   
    if(byNewName==1){shortCut(memory.m_nameA, iNewName);}
    else if(byNewName==2){shortCut(memory.m_nameB, iNewName);}
    else if(byNewName==3){shortCut(memory.m_nameC, iNewName);}
    else if(byNewName==4){shortCut(memory.m_nameD, iNewName);}
    else if(byNewName==5){shortCut(memory.m_nameE, iNewName);}
    else{iNewName = 1;}
  } // end of loop

} // end of SetName function

 void main ()
 {
    while(1)
    {
      if (Button_pushed)
      SetName(GetName+1);
    } // end of infinite loop
 } // end of main

Answer 3

Firstly, I advice you to avoid global variable if you have the choice. It's more readable.
But, here after a solution by using pointers (and global variables with your logic):

#include <string.h>
static int iName = 0;
static int iNewName = 0;
static int nbNames = 5; /* can be changed */
static char ** tableName = NULL;


BYTE GetName ()
{
 return  iName;
}


void shortCut(char* m, int newNameNumber)
{
   if (strlen (m) == 0)
     new_name++;
   else
     iName = newNameNumber;
}

void SetName (int iNewName)
{

  while (iName != iNewName)
  {   
    if(byNewName > 0 && byNewName <= nbNames) {
      shortCut(tableName[byNewName-1],iNewName);
    } else {
       iNewName = 1;
    }
  } // end of loop

} // end of SetName function


 void main ()
 {
     tableName = (char**)malloc(sizeof(char*)*nbNames);
    /* here check tableName != NULL */

    /* I don't know which data structure is 'memory' but you should initialize the value with a loop*/
    tableName[0] = memory.m_nameA;
    /* ... */

    while(1)
    {
      if (Button_pushed)
      SetName(GetName+1);
    } // end of infinite loop
    free(tableName);
 } // end of main