How is the proper way to use class members in another class?

Question

I am trying to use class members from a different class adding the "class uart" to "class parser : public uart"

But I can't use members variables in parser class for example (bufer_size, buffer, or state Enum the values are not changing or updating) the only way I can use these variables are using them as external variables, but I would like do it in a class member way.

Below please find my code:

main:

#include <stdio.h>
#include "parser.h"
int main()
{
    parser m_parser;
    m_parser.test();
    return 0;
}

uart.h

#include <stdint.h>
#include <stdio.h>

//extern uint16_t buffer_size;
//extern char buffer[1024];

class uart
{
public:
    // Construction
    uart();

    //..... some functions
    void Initialize();

    //members
    enum STATE_enum
    {
        Buffering_message=0,
        Message_received,
        Buffering_empty
    };
    STATE_enum state;

    uint16_t buffer_size;
    char buffer[1024];
protected:
    //  static void UARTHandler(app_uart_evt_t * p_event);
    void Message();
    // Singleton instance
    static uart * m_instance;
};

uart.cpp

#include "uart.h"
//uint16_t buffer_size=0;
//char buffer[1024];

uart * uart::m_instance = 0; // Singleton instance

uart::uart()// Construction
{
    state = Buffering_empty;
    m_instance = this;
}

void uart::Initialize()
{
}
/*void UART::UARTHandler(app_uart_evt_t * p_event)
{
    Message();
}*/
void uart::Message()
{
    uint8_t value;
//  while ((app_uart_get(&value)) == NRF_SUCCESS) //specific function from my microcontroller stack for reading RX bytes
    {
        if(value == 0x0A ) // message end /r
            state = Message_received;
    }       
    switch (state)
    {
        case Message_received:
                printf("message:[%s] buffer_size:[%d]", buffer,buffer_size); //printf fine from there
        break;      
        case Buffering_message:
                buffer[buffer_size++] = value;              
        break;
        default:
        break;      
    }       
}

parser.h

#include <stdio.h>
#include <stdint.h>
#include "uart.h"

class parser : public uart
{
public: 
    parser();

    void test();
    static parser * m_instance; 

    static inline parser & Instance()// Singleton access
    {
        return *m_instance;
    }       
};

parser.cpp

#include "parser.h"

class parser * parser::m_instance = 0;

// Constructor
parser::parser()
{
    m_instance = this;
}

void parser::test()
{       
    printf("state %s", state); // sending AT command

//  memset(buffer, 0, buffer_size); 
//  buffer_size = 0;    
}

Should I use the above as a Public class or add it as a friend class, or simply use as external variables?

Answer 1

Initial problem apparently was using a bad format specifier:

void parser::test()
{       
    printf("state %s", state);
    //            ^^
}

%s is for C strings only (i. e. null terminated char arrays). Here, state is interpreted as a pointer, but it points to invalid addresses (undefined behaviour), so you're application might have crashed before it was able to produce any output at all.

Even if state did contain a valid address, you would still suffer from undefined behaviour because %s requires an argument of type char* (to avoid UB, you need to cast).

Use the correct format specifier to get around the problem:

printf("state %u", static_cast<unsigned int>(state));

You still need the cast to avoid UB as %u requires unsigned int, but your enum is not (alternatively you could use %d and cast to int).

Apparently, too, you now tried (because of not having discovered the actual error) to get around what you assumed it to be by use of singletons – unfortunaly, by badly implementing the pattern, you flawed your design.

How to repair? At first, drop the singletons. There absolutely is no need for in given case. Then think twice (the following already was an issue before): Is a parser really an UART? Not considering the code, but the concepts, I clearly say "no". So you should not let inherit parser from uart either. Rather let it aggregate one:

class parser // : public uart
{
    uart m_uart;
public:
    // ...
    void test()
    {
        printf("%u", static_cast<unsigned int>(m_uart.state);
    }

};

Alternatively, you might provide it externally via pointer or reference (to the constructor) and store it as such, but then you will quickly run into life time management as well, more advanced concepts I'd recommend you to come back to with a little bit more of experience...

What you should get used to, though: encapsulation. Don't make member variables public unless you really want them to be changed from anywhere. For instance, the buffer is a hot candidate for being private (not even protected).

Finally buffer_size: If you intend this to be used to count number of values currently in buffer, then it is fine. If it shall represent the maximum number of data that can be stored, then it is redundant. Wherever you need it, you can get the size by sizeof(buffer)/sizeof(*buffer) (unless the array has decayed to pointer!). Division by size of first element is necessary as sizeof always delivers size in bytes, not in number of elements.

Size in number of elements is what you get by std::array::size; you should prefer std::array over raw arrays due to the superior interface! Maybe std::vector even is a yet better choice?