[riot-notifications] [RIOT-OS/RIOT] Refactor evtimer add del (#10717)

Kees Bakker notifications at github.com
Wed Jul 3 18:30:55 CEST 2019


keestux commented on this pull request.



>  {
     DEBUG("evtimer: new event offset %" PRIu32 " ms\n", event->offset);
-    /* we want list->next to point to the first list element. thus we take the
-     * *address* of evtimer->events, then cast it from (evtimer_event_t **) to
-     * (evtimer_event_t*). After that, list->next actually equals
-     * evtimer->events. */
-    evtimer_event_t *list = (evtimer_event_t *)&evtimer->events;
-
-    while (list->next) {
-        evtimer_event_t *list_entry = list->next;
+    evtimer_event_t **list = &evtimer->events;

I'm in favor of easy to understand code. But I still prefer my version. What else can I do to convince you? Shall we measure code complexity (McCabe?), or shall we measure code size? Do we need more comments (it's basically a simple while-loop), explaining the iteration. ASCII art?

Leaving out DEBUG, it comes down to the following.
```
static void evtimer_add_event_to_list(evtimer_t *evtimer, evtimer_event_t *event)
{
    evtimer_event_t **list = &evtimer->events;

    while (*list) {
        /* Stop when new event time is nearer then next */
        if (event->offset < (*list)->offset) {
            break;
        }
        /* Set event offset relative to previous event */
        event->offset -= (*list)->offset;
        list = &(*list)->next;
    }

    /* Set found next bigger event after new event */
    event->next = *list;
    if (*list) {
        /* Set offset following event relative to new event */
        (*list)->offset -= event->offset;
    }
    *list = event;
}
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/10717#discussion_r300052345
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190703/279fe7d9/attachment.html>


More information about the notifications mailing list