Code review comment for lp:~dandrader/frame/smarter_backend

Revision history for this message
Chase Douglas (chasedouglas) wrote :

* In frame_backend_frame_borrow_touch_by_id(), if it fails because the touch ID doesn't exist, the touch pointer is reset to NULL. This should be nullptr instead of NULL, but I would argue it shouldn't be reset at all. One technique often employed by APIs is that either a function succeeds, or it fails and does not modify any state. It can be hard or practically impossible to do this in some circumstances, but here it is as simple as not resetting the touch pointer value. I can't give you any specific scenario under which this particular change would be helpful, but I prefer the API to behave like this as much as possible.

* In frame_backend_frame_give_touch(), I think *touch should be set to nullptr instead of 0. Or maybe I'm not realizing something that makes that not work?

Otherwise, I'm ok with everything. It's not exactly how I would do it, but I don't know of a 100% best alternative either.

review: Approve

« Back to merge proposal