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

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> On 08/30/2012 11:01 AM, Daniel d'Andrada wrote:
> >> * Any functions that may fail need to return a UFStatus and provide the
> return
> >> result through a referenced parameter. All of the new functions here should
> do
> >> this. I forgot about this when I approved the previous merge proposal as
> well.
> >> We should go back and fix these up.
> >
> > Isn't that extra status variable redundant in those cases? What's the
> benefit?
>
> Let's take a function like:
>
> UFBackendTouch frame_backend_frame_get_touch_by_id(
> UFBackendFrame frame, UFTouchId id)
>
> Without knowing exactly how this is implemented, there are two possible
> recoverable error conditions:
>
> 1. The touch ID is invalid (UFStatusErrorInvalidTouch)
> 2. The allocation and initialization of UFBackendTouch could fail
> (UFStatusErrorResources or UFStatusErrorGeneric)
>
> The current implementation doesn't actually catch allocation and
> initialization exceptions, but it could if we found that to be a problem.
>
> The client could use this information to determine how to proceed. It's
> almost impossible to recover from allocation errors, so the client could
> abort. However, if the touch ID is invalid, it could stop processing
> gestures for that frame and continue on. At the very least, it could log
> a more meaningful error message, which has been helpful in debugging
> issues in the past.

You're right.
Thanks for the explanation!

« Back to merge proposal