[vlc-devel] [PATCH 01/14] mkv: introduced helper functions related to indexes
filip at videolabs.io
Thu Mar 10 14:03:24 CET 2016
On 16/03/10 12:00, Filipe Cabecinhas wrote:
> Why replace "indexes.begin()", which is pretty standard with
> "indexes_begin()", which has more cognitive load with seemingly no gain?
To make it have "symmetry" with `indexes_end()`.
The problem boils down to the previous code using the container in a rather
weird way, and in order to not having to change all logic associated with the
indexes (cues) prior to a (much likely) rewrite of everything that has to do
with indexes, it was decided to simply wrap the old behavior.
As an example, when we first create the index container it will always contain
one (`1`) index (due to legacy logic). That index is however not to be used when
seeking, but only to act as a placeholder for code that needs to read/write some
state depending on that index.
The previous code used the last element (ie. `container.end()-1`) as this
placeholder, and this continues to be true after the rewrite. In order to not
get confused by code such as:
indexes_t::const_iterator cit = indexes.begin();
while (cit != indexes_end())
Where one could think that using `indexes_end()` together with `indexes.begin()`
is a mistake, it was decided to introduce `indexes_begin()` to make it really
feel like they belong together.
However, one could have also changed the name of `indexes_end()` to something
like `active_indexes_end()`, or something.. similar (naming is hard)--but that
did not happen.
I would welcome a patch that clears up the naming of the two, though as said the
entire segment will most likely be rewritten quite soon.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel