0

I have a map of std::shared_ptr's for both key and value and I'm trying to find the right element. Below is what I have, though, it still has a problem and won't compile.

std::map<std::shared_ptr<MyObjA>, std::shared_ptr<ValObj>> aobjs;
std::map<std::shared_ptr<MyObjB>, std::shared_ptr<ValObj>> bobjs;

template <typename T>
int findit(T& t)
{
    T myobj = t;

    typename std::map<std::shared_ptr<T>, std::shared_ptr<ValObj>>::iterator it;

    if constexpr(std::is_same<T, net::MyObjB>::value) {
        net::MyObjB objB;
    
        // failing here ...
        auto it = std::find_if(bobjs.begin(), bobjs.end(), 
                [&](std::shared_ptr<MyObjB> const& p) {
            return *p == objB;
        });

        if (it != bobjs.end()) {
            // found it
        
            return 0;
        }
    }
    
    return -1;
}

I think the problem is with the first and last of the find_if returning the std::shared_ptr, but unsure. I got a ton of output that I've had trouble parsing ...

/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/include/g++-v10/bits/predefined_ops.h: In instantiation of ‘constexpr bool __gnu_cxx::__ops::_Iter_pred<_Predicate>::operator()(_Iterator) [with _Iterator = std::_Rb_tree_iterator<std::pair<const std::shared_ptr<MyObjB>, std::shared_ptr<ValObj> > >; _Predicate = findit<MyObjB>::<lambda(const std::shared_ptr<MyObjB>&)>]’:
/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/include/g++-v10/bits/stl_algobase.h:1912:42:   required from ‘constexpr _InputIterator std::__find_if(_InputIterator, _InputIterator, _Predicate, std::input_iterator_tag) [with _InputIterator = std::_Rb_tree_iterator<std::pair<const std::shared_ptr<MyObjB>, std::shared_ptr<ValObj> > >; _Predicate = __gnu_cxx::__ops::_Iter_pred<findit<MyObjB>::<lambda(const std::shared_ptr<MyObj>&)> >]’
/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/include/g++-v10/bits/stl_algobase.h:1974:23:   required from ‘constexpr _Iterator std::__find_if(_Iterator, _Iterator, _Predicate) [with _Iterator = std::_Rb_tree_iterator<std::pair<const std::shared_ptr<MyObjB>, std::shared_ptr<ValObj> > >; _Predicate = __gnu_cxx::__ops::_Iter_pred<findit<MyObjB>::<lambda(const std::shared_ptr<ValObj>&)> >]’
/usr/lib/gcc/x86_64-pc-linux-gnu/10.3.0/include/g++-v10/bits/stl_algo.h:3934:28:   required from ‘constexpr _IIter std::find_if(_IIter, _IIter, _Predicate) [with _IIter = std::_Rb_tree_iterator<std::pair<const std::shared_ptr<MyObjB>, std::shared_ptr<ValObj> > >; _Predicate = findit<MyObjB>::<lambda(const std::shared_ptr<MyObjB>&)>]’

Thoughts?

Ender
  • 1,652
  • 2
  • 25
  • 50
  • 3
    Please [edit] and show a [mcve] – Jabberwocky Nov 15 '21 at 21:38
  • This function signature is not correct: `int findit`. That should be a pretty easy error to fix. – Ted Lyngmo Nov 15 '21 at 21:41
  • @TedLyngmo - yeah, typo, fixed. – Ender Nov 15 '21 at 21:42
  • 1
    `template `...? `std::shared_ptr`? What is `VObj`? It'd be better if you actually made a [mre] and _copied_ it into the question. – Ted Lyngmo Nov 15 '21 at 21:44
  • Working it now, it'll take me a bit. – Ender Nov 15 '21 at 21:46
  • 1
    `std::find_if` requires a predicate that accepts the `value_type` of the container searched. The _value type_ of `std::map` is `std::pair`. – Drew Dormann Nov 15 '21 at 21:51
  • `std::map` has its own [`find()`](https://en.cppreference.com/w/cpp/container/map/find) method to search for a key. Don't use `std::find_if()`. Just note that `std::map`'s default compare requires the key type to support `operator<`, which `std::shared_ptr` supports prior to C++20, but `operator<` is removed for `std::shared_ptr` in C++20. – Remy Lebeau Nov 15 '21 at 22:06

1 Answers1

2

I have a map of std::shared_ptr's for both key and value

... thoughts?

A few things come to mind:

1. It is likely not a good idea to hold a map like that - especially with respect to the keys. It doesn't make sense to have "heavy" keys, that require resource allocation, and which you are likely avoiding holding many copies of.

It is likely that MyObjA's have some kind of cheap numeric identifier you can use; and if they don't - consider adding such a field, and constructing MyObjA with such a unique identifier. Then, instead of a map from MyObjA's to MyObjB's, you could map from ObjAKey to MyObjB's, or even from ObjAKey to std::pair's of a MyObjA and MyObjB.

2. I suspect that even for the map values, a std::shared_ptr may not be necessary, but you haven't given us enough context.

3. Do you really need an ordered map? Would std::unordered_map not work for you? Also, if the number of elements in your map is not big enough, you can probably just use a vector of MyObjA, MyObjB pairs and it'll be good enough. It's not as though the standard library maps are fast.

einpoklum
  • 118,144
  • 57
  • 340
  • 684