Search code examples
cencapsulationnaming

How to name a good/meaningful type?


Device_Manager.h

typedef enum
{
    DNM = 0x2A,

}TYPE_e;

typedef struct DEVICE_s* p_DEVICE;
typedef p_DEVICE(*FUNC)(char* name, TYPE_e type, uint32_t ip, uint16_t method, uint16_t groupRule);   

p_DEVICE DeviceManager_New(void);
p_DEVICE DeviceManager_Ctor(char* name, TYPE_e type, uint32_t ip, uint16_t method, uint16_t groupRule);
p_DEVICE DeviceManager_Dtor(p_DEVICE element);

Device_Manager.c

struct DEVICE_s
{
    uint32_t IP;
    TYPE_e Type;
    uint16_t Method;
    uint16_t GroupRule;
    char Name[40];
    FUNC fp_Ctor, fp_Dtor;    //this line needs modification
}DeviceSet[32],DeviceTemp;

p_DEVICE DeviceManager_InitObject(p_DEVICE* self)
{
    (*self) = DeviceManager_New();
    (*self)->IP = 0;
    (*self)->Type = 0;
    (*self)->Method = 0;
    (*self)->GroupRule = 0;
    memset((*self)->Name, 0, NAME_SIZE);
    (*self)->fp_Ctor = DeviceManager_Ctor;
    (*self)->fp_Dtor = DeviceManager_Dtor;    // warning: assign to wrong type
    return (*self);
}
p_DEVICE DeviceManager_New(void)
{
    return &DeviceTemp;
}
p_DEVICE DeviceManager_Ctor(char* name, TYPE_e type, uint32_t ip, uint16_t method, uint16_t groupRule)
{
    memcpy(DeviceTemp.Name, name, sizeof(name));
    DeviceTemp.Type = type;
    DeviceTemp.IP = ip;
    DeviceTemp.Method = method;
    DeviceTemp.GroupRule = groupRule;
    return &DeviceTemp;
}
p_DEVICE DeviceManager_Dtor(p_DEVICE element)
{
    element->IP = 0;
    element->Type = 0;
    element->Method = 0;
    element->GroupRule = 0;
    memset(element->Name, 0, NAME_SIZE);
    return element;
}

This is my first time implementing encapsulation concept and came across some problem.

In header file, I used typedef to define type "FUNC" as a function pointer.

I think this name "FUNC" is not clear enough, because this naming fashion will result in:

struct DEVICE_s
{
    uint32_t IP;
    TYPE_e Type;
    uint16_t Method;
    uint16_t GroupRule;
    char Name[40];
    FUNC1 fp_Ctor;    //not clear
    FUNC2 fp_Dtor;    //not clear
}DeviceSet[32],DeviceTemp;

fp_Ctor and fp_Dtor are both same type(function pointer) and differ in argument's number.

I always struggle in naming type. Can offer some suggestions on naming type?


Solution

  • This is all a bit subjective, but I would start with dropping the style of hiding pointers behind typedef and smear some manner of "Hungarian notation" over it. A whole lot of C programmers will agree there.

    So the first suggestion is to go with

    typedef struct DEVICE_s DEVICE_s;
    

    And then define your opaque interface based on DEVICE_s* instead. Not only is it easier to read, you filter out obfuscation like a caller attempting to pass p_DEVICE* to the user-defined functions etc, because they don't realize they already have a pointer. (Win32 API suffers heavily from this problem.)

    Then your constructor becomes

    DEVICE_s* DeviceManager_Ctor ( ...
    

    And all member functions will take a DEVICE_s* parameter rather than a p_DEVICE by value. The caller will have to declare pointers instead of objects, making it clear to them that they have a pointer to incomplete type and nothing they can/should play around with.

    Next up you can drop the pointer hiding in the function pointer too. This is less of an issue, but it is nice to be consistent:

    typedef DEVICE_s* DeviceManager_Ctor_t ( ...
    

    Your function pointer definitions would then become:

    DeviceManager_Ctor_t* Ctor;
    

    You can drop the "fp" naming as it is already obvious that the type is a function pointer.


    As a side note, I would recommend to avoid mimicking C++ member functions with obj.member notation. Because in C, lacking a this pointer, you will end up with obj.member(&obj, ...) which is kind of redundant.

    Rather just accept that C is the way it is and call member functions as DeviceManager_Ctor(obj); where obj is declared as DEVICE_s* obj;. The key to readable OO code is to use a consistent source code prefix for all functions belonging to the "class", as you already do: DeviceManager_.