Streamlining Code - Multiple Declarations and Check Logic

@ScruffR, You made a comment about code in another post here .

if you would not mind. I would like to see your implementation of that code in the way you were talking about please. Below is a clip of what you said, and the code.

Not addressing your actual problem, but one hint I usually and hope at one point people will pick up on:
Instead of having tons of individual variables like here

// PUB-SUB constants
constexpr char * DEVICE_NAME = "ALERT";
constexpr char * WINDOWSHUT_MSSG = "Office window shut.";
constexpr char * WINDOWOPEN_MSSG = "Office window open.";
constexpr char * BWINDOWSHUT_MSSG = "Bathroom window shut.";
constexpr char * BWINDOWOPEN_MSSG = "Bathroom window open.";
constexpr char * BDSHUT_MSSG = "Bathroom door shut.";
constexpr char * BDOPEN_MSSG = "Bathroom door open.";
constexpr char * SDCLOSED_MSSG = "Sliding door closed.";
constexpr char * SDOPEN_MSSG = "Sliding door open.";
constexpr char * KEEPBOLT_MSSG = "Stores door locked.";
constexpr char * KEEPOPEN_MSSG = "Stores door unlocked.";
constexpr char * MDKEEPBOLT_MSSG = "Main door locked.";
constexpr char * MDKEEPOPEN_MSSG = "Main door unlocked.";
constexpr char * RSATUS_MSSG = "Requesting Status";
constexpr char * SHOPL_MSSG = "SHOPLIFTER";
constexpr char * RESCUE_MSSG = "RESCUE";
constexpr char * RAIN_MSSG = "RAIN";

and a rats nest of check logic like this

        if (strcmp(data, SHOPL_MSSG) == 0) {
                myDFPlayer.playMp3Folder(1);
        } else if (strcmp(data, RESCUE_MSSG) == 0) {
                myDFPlayer.playMp3Folder(4);
        } else if (strcmp(data, RAIN_MSSG) == 0) {
                myDFPlayer.playMp3Folder(20);
        } else if (strcmp(data, MDKEEPOPEN_MSSG) == 0) {
                maindoorlocked = 1;
        } else if (strcmp(data, MDKEEPBOLT_MSSG) == 0) {
                maindoorlocked = 0;
        } else if (strcmp(data, SDOPEN_MSSG) == 0) {
                slidingstate = 1;
        } else if (strcmp(data, SDCLOSED_MSSG) == 0) {
                slidingstate = 0;
        } else if (strcmp(data, KEEPOPEN_MSSG) == 0) {
                Storeskeepstate = 1;
        } else if (strcmp(data, KEEPBOLT_MSSG) == 0) {
                Storeskeepstate = 0;
        } else if (strcmp(data, BWINDOWOPEN_MSSG) == 0) {
                bathroomwindowlocked = 1;
        } else if (strcmp(data, BWINDOWSHUT_MSSG) == 0) {
                bathroomwindowlocked = 0;
        } else if (strcmp(data, WINDOWOPEN_MSSG) == 0) {
                officewindowlocked = 1;
        } else if (strcmp(data, WINDOWSHUT_MSSG) == 0) {
                officewindowlocked = 0;
        }

Thanks for your time.

This is not a one-to-one translation of the code, but should illustrate what I’m after

struct STATES {
  const char text[16];                              // verbal description of state
  const int  value;                                 // numeric value for that state
} state[] =
{ { "shut"    , 0 }
, { "locked"  , 0 }
, { "closed"  , 0 }
, { "unlocked", 1 }
, { "open"    , 2 }
};
const int numStates = sizeof(state) / sizeof(state[0]);

struct OBJECTS {
  const char text[32];                              // verbal description of the object
  int        state;                                 // current state of that object 
} object[] = 
{ { "Office window"  , 0 }
, { "Bathroom window", 0 }
, { "Bathroom door"  , 0 }
, { "Sliding door"   , 0 }
, { "Stores door"    , 0 }
, { "Main door"      , 0 }
};
const int numObjects = sizeof(object) / sizeof(object[0]);

void handler(const char* event, const char* data) {
    int s, o;
    
    for (int s = 0; s < numStates; s++)             // find match between data state description
      if (strstr(data, state[s].text)) break;       //   if found stop search     
    if (s >= numStates) s = -1;                     // no valid state found in string

    for (int o = 0; o < numObjects; o++)            // find match between data and object name
      if (strstr(data, object[o].text)) break;      //   if found stop search      
    if (o >= numObjects) o = -1;                    // no valid object found in string
    
    if (o >= 0 && s >= 0)
      object[o].state = state[o].value;             // set the current state of object accordingly
    else 
      Serial.printlnf("'%s' is errors: %s%s", data, (o < 0) ? "unkonwn Object " : "", (s < 0) ? "unknown State " : "");
}

With that kind of logic, you can add states and objects just to the arrays without ever having to revisit the functional code. You also reduce the risk of copy/paste errors, where you just fail alter that one of all these very similar looking code blocks - and these are hell to find when your code just doesn’t behave as expected, since you read over it time and time again as everything looks just too similar.

With some planing ahead and minimal restructure of your variable set, you can save yourself a lot of hassle typing, debugging and extending your code in future.

2 Likes

WOW, I also tried to comprehend your concept on the other post.

Thanks so much for the detailed explanation @ScruffR and thanks for asking @mikemoy !

This is extremely helpful.

1 Like