Document 'Collection' comparison with 'Container' named requirement #598
Conversation
…ence_type` required by container
…ases to `UserDataCollection`
…docs, add more checks
Co-authored-by: hegner <[email protected]>
Co-authored-by: tmadlener <[email protected]>
…ad of ! (as suggested in catch2 docs)
m-fila
force-pushed
the
collection_container
branch
from
9850736
to
e25de51
Compare
Yes, please 🙂 I realized that we have a detection idiom implemented in utils. I could rewrite the traits in test with it to reduce the boiler plate a bit if you think it's worth it |
|
No strong opinion on the usage of the detection idiom, I leave it up to you. |
There was a problem hiding this comment.
Thanks again for putting this together. I like this quite a lot, and I have mainly smaller comments (see below).
One thing that I am not sure how to handle best yet is the fact, that you stayed entirely within the STL language of using iterator and const_iterator. This makes this document self contained and it is straight forward to compare to the STL and the named requirements. However, there is no mention yet of the Iterator and MutableIterator that we generate. Do you think there is an easy way to at least mention them somewhere. I think that could reduce the potential for confusion if people look e.g. at the doxygen API documentation for EDM4hep.
fix typos fix swap docs clarify iterator as CollectionIterator fix destructor behaviour docs add standard algorithm links and warnings Co-authored-by: tmadlener <[email protected]>
Thank you very much 😊 As for the detection idiom I think I'll leave it as is |
There was a problem hiding this comment.
I am happy with this as it is. In case we really want to go through all > 100 STL algorithms and document them we can do that later as well, but for now this should serve as a very good basis and we have something that we can refer to for capabilities.
@hegner do you want to have another look?
|
I heard no complaints, so I will merge this. |
|
Thanks! |
BEGINRELEASENOTES
cbegin,cend,max_sizeand aliasessize_type,difference_typeto collectionENDRELEASENOTES
Fixes #479, Fixes #546
The goal of this PR is to document current behavior not to fix things. It'll be easier to fix later once we known what is missing. A few methods and aliases were added but they don't collide with anything else
C++ specifies Container as a named requirement, that means specifies required types, functions, semantics and behaviors as well post requirements on its iterators.
The requirements were collected in a form of a markdown file. Next to each requirement it's indicated whether it's fulfilled by the collection/iterators. A short comment indicates any peculiar behaviors or why it isn't fulfilled.
The document is accompanied by a set of tests. The tests are meant to detect changes in collection semantic and interface, and fail indicating need to update the documentation
Beside Container a short paragraph and a few tests are added for checking AllocationAwareContainer (collection uses default allocator implicitly #424), iterator adapters. Another short paragraph is added on interactions with standard algorithms (algorithms require at least LegacyInputIterator which is not fulfilled by collection iterators, the code may compile but it's std-implementation depended #150) and ranges (which use at least std::input_iterator constrain that again isn't fulfilled by collection iterators #273)