slashback100 / presence_simulation

Home Assistant Presence Simulation
423 stars 22 forks source link

Update how events are removed from list. #84

Closed blester125 closed 1 year ago

blester125 commented 1 year ago

Currently, when events are removed from the _next_events list it uses a for loop and del. This modifies the list during iteration and results in the possibility of missing an event to be deleted. In the similar implementation below, we see that the first and last 3 are removed but the second one in the run is not.

In [1]: a = [1, 2, 3, 3, 4, 6, 3]
In [2]: def remove(x, l):
   ...:     i=0
   ...:     for e in l:
   ...:         if e == x:
   ...:             del l[i]
   ...:         i += 1
In [3]: remove(3, a)
In [4]: a
Out[4]: [1, 2, 3, 4, 6]

This PR updates the removal method by using a list comprehension and if to filter out events based on entity_id.

Note: The comment and the implementation seem to imply different things. The comment says to delete the next event for an entity while the implementation deletes all events (ignoring the bug above) for an entity. If the semantics of the comment are correct this PR should not be merged and instead a break should be added after the del statement.

slashback100 commented 1 year ago

Actually in the _next_events, I will have each entity only once. So indeed a break would be a good idea, and you proposition will also work. I'll keep your version, which is prettier :-) Thanks