Instant card updates reset view previously modified by the user
Opened this issue · 2 comments
Consider the following scenario:
The user starts a sequence of +1 clicks on some card empty card, that is, a card with currently 0 absences, the first will instantly trigger as expected (see TrialScheduler and AttendanceCard), yielding 1 absence to this card. The next ones will constantly re-schedule the call to the appropriate listener (AttendanceCard L.72) until the user gives a pause between his clicks of something more than the INTERVAL_UPDATE_TRIGGER (AttendanceCard L.25), but less than 2 times it (think of one click happening t seconds before the next update, it will re-schedule this update for more u seconds, so the time the user must wait for this update to be triggered is u + t, for which holds: u < u + t < 2*u since 0 < t < u). Let's say he decided to stop clicking when there was 15 absences in the respective card. But unfortunately, our user won't simply wait 'till the update to the database (which is what the real listener do, see AttendanceCardAdapter L.74), the user fires another click to +1 of ANOTHER card, which naturally enough will instantly trigger an update (since it's the first click), thus happening before the first update that has been re-scheduled a couple of times. The fact is that there is no mechanism to handle those collisions, so, the result is that this intrusive update will re-render the former card and reset its absences to 1 (which was the last modification to the database coming from the first card), what happens behind the curtains is that there is no locking to the re-scheduling mechanism, it just re-schedules the task, however this task uses an instance variable, namely mAttendance (see parameter on method call aat AttendanceCall L.72). So, before our update on hold gets called, the re-render (which is a result from the loader at AttendanceFragment L.166 and AttendanceCardAdapter L.69) will end up at AttendanceCardAdapter L.125 -> AttendanceCard L.92, resetting the internal variable mAttendance. Thus, when the time arrives for the update, it will be triggered, but the object passed will no longer be the one expected before (which probably this time has already been garbage collected, or not =P), that is, on AttendanceCard L.72 mAttendance is the new fresh object received from the database, so the update won't have the desired effect of setting the absences to 15. Unfortunately, this is not the only problem, after the intrusive call (a.k.a. the hated click of the user), the user will be negatively surprised with a UI refresh that will reset the absences of the first card to 1 abruptly without his requesting it nor expecting it, which is really bad.
A few solutions to the problem were thought and I'll sketch some of them here. The first one was to create a middle man between the DB updates (which are in fact quite independent, and, indeed, you can feel the need for a ThreadPool or something similar) to enqueue them, and some sort of mechanism for the cards to tell this middle man to request resources and release them (the implementation could be a simple counter), and this middle man would onlycommit the updates to the DB once all resources have been released (which would imply the user is not clicking anymore in any +1 or -1, or whatever that triggers DB updates). One problem with this approach is that it no longer follows the appreciated pattern on in-lineeditions of android, that as soon as you leave the focus of some command, its modificationis stored. Another possible problem is that, although in the present the only way to triggerupdates are through this modifications and edition/creating of new courses, in the futuredb updates could come from different sources and the problem of some update modifying a cardwhich the user is currently interacting with would continue and result in unpleasant experiences to the user. So the slight favored solution is the second. It also consists of having a middle man, but not in the model side, rather in the view side. It would be an object between the AttendanceFragment and the AttendanceCardAdapter, possibly it could be a object playing similar role of the AttendanceCardAdapter but composing one of BaseAdapter,instead of inheriting from it. It would also give resources to cards and let them releasethose, but it wouldn't manage db operations, it would manage view renders. In other words,it would only update the whole list of cards once all current modifications have finished,so it would have to enqueue view updates (perhaps in the form of the data received, thelists of Attendances at AttendanceFragment L.165) and dequeue* and render them when all resources have been released (i.e. the uses is no longer clicking in critical buttons). Finally, it's worthy saying that AttendanceCard would have to have a reference to thismanager in order to be able to release his requested resources when the callback gets called.
- Maybe only consider the last one.
As noted on AttendanceCardAdapter on branch cardupdatefix.
Another options is to simply created a mechanism for the cards to dismiss future updates comming from the DB (and perhaps some sort of identification included, to dismiss only those updates comming from their triggers), this is a way of telling the AttendanceFragment that the card is also responsible for modifying its view.