> Every single comment I have made about accessor functions vs generic property > getter/setter applies here too. In fact it applies even stronger. As an > example, the Axis class has no use for a generic setter. None. Zilch. Zero. > There are no new properties that can magically appear at runtime. They are all > specified by the kernel interface, the type enum and they are all required > arguments of the constructor. All it does is complexify the code and > circumvent the type system making the entire thing more fragile. Note that there are no setters in the api. One can reason that the axis object is not going to be extended, so I have converted frame_axis_get_property() into four separate static accessor function. The rest of the objects may be extended in the future, and they could be extended at runtime. Leaving them as generic opaque getters allows for better forward compatibility. > And if opaque getters and setters are to be used, they better be used > everywhere and always. For example, UFTouchClass has getters and setters for > id and state but not others. Which is it gonna be? Touches are retrieved through an indexed lookup because there's no other way to retrieve them from the frame object. Axes can be retrieved through an indexed lookup for the same reason until you know what axis types are available for the device. Once you have looped through the available axes you can get an axis directly by its type. These are special cases because the objects are part of arrays. > UFHandleClass requires an Initialize call before it is usable. This is wrong. > Either this needs to be done in the constructor or a static factory. It might > be sensible not to use exceptions in this case since Frame is being used as a > C library. I'm not quite sure what happens if an exception escapes the > boundary. It has been converted to a factor function. Additionally, frame_x11_new() now catches any exceptions and returns UFStatusErrorResources. > There are multiple duplicate declarations of SharedUFDeviceClass, > SharedUFDeviceClass etc. There can be only one. These are essentially forward declarations of the typedefs. If you don't forward declare them, you will need to include the appropriate header files. If you attempt to do that (which I tried to do), you get into a header recursion and end up with undefined type errors. This leaves us with two possibilities: forgo the typedefs and use the exact definitions, or leave the forward declarations of typedefs. Note that the typedefs cannot get out of sync or else you will get compiler errors stating there is a conflicting declaration. I have decided to leave them as it makes the code easier to read. > The PutEvent/GetEvent interface is a bit cryptic. Why is it called PutEvent if > it just deletes the event. Should it not be ReleaseEvent? "Put" in a container > object implies adding new stuff for further use. Actually, the entire > interface seems a bit fishy to me. Could you give an example on how you > envision it will be used and how it differs from the current API. In linux, and I believe elsewhere too, some reference counting APIs use "get" and "put" to increment and decrement the reference counter. I used the naming scheme here because when you get an event you are similarly getting ownership of the event, and when you put it you are putting ownership back and the event is freed. However, I do like the name "release" more, so I have modified the api as you suggested. If you want a brief idea of how the api is used, take a look at the utouch-frame-test-x11 program. Imagine instead of printing out events you take events and feed them into a gesture recognizer. It's as simple as that. > If this is going to be C++0x, then these sorts of structures: > > for (unsigned int i = 0; i < prev_->touches_array_.size(); ++i) { > const SharedUFTouchClass& prev_touch = prev_->touches_array_[i]; > > should be replaced with auto iterators. Good point :). I'm new to c++0x, so I had forgotten about the auto keyword. I have switched over all iterators.