mattreecebentley / plf_list

A drop-in replacement for std::list with 293% faster insertion, 57% faster erasure, 17% faster iteration and 77% faster sorting on average. 20-24% speed increase in use-case testing.
https://plflib.org/list.htm
zlib License
151 stars 21 forks source link

non-const unordered_find_single() #16

Closed degski closed 4 years ago

degski commented 4 years ago
iterator unordered_find_single(const element_type &element_to_match) PLF_LIST_NOEXCEPT

I would expect this function to be marked const, if it [the function] cannot be marked const because of some internal (state?), the function should be declared const and the internal (state) mutable.

This function must be marked const.

mattreecebentley commented 4 years ago

I checked and you are correct.

degski commented 4 years ago

Thanks.

mattreecebentley commented 4 years ago

Actually I'm having second thoughts about this - with begin() we only make it const if it's returning a const iterator - not sure if there's some language guidelines here.

degski commented 4 years ago

Actually I'm having second thoughts about this - with begin() we only make it const if it's returning a const iterator - not sure if there's some language guidelines here.

I get you are as confused as I was before I sorted it out for my-self. Below are the required interfaces. So, cbegin() const is an alias for begin() const. There are possibly others of course like reverse ones.

[[nodiscard]] nodes_const_iterator begin ( ) const noexcept { return nodes.begin ( ); }
[[nodiscard]] nodes_const_iterator cbegin ( ) const noexcept { return nodes.cbegin ( ); }
[[nodiscard]] nodes_iterator begin ( ) noexcept { return nodes.begin ( ); }

[[nodiscard]] nodes_const_iterator end ( ) const noexcept { return nodes.end ( ); }
[[nodiscard]] nodes_const_iterator cend ( ) const noexcept { return nodes.cend ( ); }
[[nodiscard]] nodes_iterator end ( ) noexcept { return nodes.end ( ); }

etc..

The above wraps (in this case a plf::colony). You can make it more 'auto' like:

[[nodiscard]] const_iterator begin ( ) const noexcept { return nodes.begin ( ); } <--- this one needs defining!
[[nodiscard]] const_iterator cbegin ( ) const noexcept { return begin ( ); }
[[nodiscard]] iterator begin ( ) noexcept { return const_cast<iterator> ( std::as_const ( *this ).begin ( ) ); }

(one is almost tempted to thow in a, God forbid, a macro like ITERATOR(begin) ;) ) and so on, they will be coherent and const-correct. In therms of insertion:

// insert_implementation ( iterator ); << the real mc-coy

[[maybe_unused]] nodes_iterator push ( value_type const & data_ ) {
    return insert_implementation ( nodes.emplace ( data_ ) );
}
[[maybe_unused]] nodes_iterator push ( value_type && data_ ) {
    return insert_implementation ( nodes.emplace ( std::forward<value_type> ( data_ ) ) );
}
template<typename... Args>
[[maybe_unused]] nodes_iterator emplace ( Args &&... args_ ) {
    return insert_implementation ( nodes.emplace ( std::forward<Args> ( args_ )... ) );
}

This is just an example.

For the unordered_find_single() you'll have to write the const version and do the std::as_const trick above to get the non-const version.

The problem is that you (plf::colony, sorry to personalize) have to do the const-cast, otherwise the user needs to do it, because in a const calling function, when calling non-const unordered_find_single() the this-pointer of the colony will be non-const and it won't compile, ever, unless you cast the non-constness away.

The things I wrote are all on cppref. The page I consult the most is std::vector, compare plf::colony against the api of std::vector and you'll always be good (that's the guide-line). Any container needs to implement (as a minimum) the api of std::vector, unless (for some (or any) bits) you cannot (coz it's not a vector :-) ).

Other than const-correctness, there is noexcept-correctness to consider (and get right) as well.