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?
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_
.