1

I have implemented a C++ program, which creates three different Person objects. These objects are made shared and stored in a vector.

The function getVector() takes a const std::vector<std::shared_ptr<Person>>& as input and returns a std::vector<Person*>. Each element in the result vector corresponds to the respective element in the input vector.

The function uses std::transform together with std::mem_fn.

The implementation of that function (and the corresponding call with std::cout's) doesn't work. I think that I use std::transform with std::mem_fn in a wrong way (wrong arguments, etc.).

I don't know how to make the function getVector() working.

Here is my source code so far:

main.cpp

#include <string>
#include <vector>
#include <memory>
#include <sstream>
#include <iostream>
#include <algorithm>
#include <functional>

struct Person
{
    std::string first_name;
    std::string last_name;
    int age;

    Person* getPerson(const std::shared_ptr<Person>& person)
    {
        Person* p = person.get();
        return std::move(p);
    }

    // Overloaded << operator.
    friend std::ostream& operator<<(std::ostream& os, const Person& person)
    {
        os << person.first_name << ", " << person.last_name << ", " << person.age << "\n";
        return os;
    }

    friend std::string to_string(const Person& obj)
    {
        std::ostringstream ss;
        ss << obj;
        return ss.str();
    }
};

std::vector<Person*> getVector(const std::vector<std::shared_ptr<Person>>& people);

int main(void)
{
    Person person01;
    person01.first_name = "Ashley";
    person01.last_name = "Graham";
    person01.age = 23;

    Person person02;
    person02.first_name = "Ada";
    person02.last_name = "Wong";
    person02.age = 45;

    Person person03;
    person03.first_name = "Leon S.";
    person03.last_name = "Kennedy";
    person03.age = 26;

    std::shared_ptr<Person> ps01 = std::make_shared<Person>(person01);
    std::shared_ptr<Person> ps02 = std::make_shared<Person>(person02);
    std::shared_ptr<Person> ps03 = std::make_shared<Person>(person03);

    std::vector<std::shared_ptr<Person>> peoples;

    peoples.emplace_back(std::move(ps01));
    peoples.emplace_back(std::move(ps02));
    peoples.emplace_back(std::move(ps03));

    std::cout << "\nContent of input vector:\n";
    std::cout << "====================================\n";
    for (auto p : peoples)
    {
        std::cout << *p;
    }
    std::cout << "\n";

    std::cout << "Calling function 'getVector()'...\n";

    std::vector<Person*> result = getVector(peoples);

    std::cout << "\nContent of result-vector:\n";
    std::cout << "====================================\n";
    for (auto r : result)
    {
        std::cout << *r;
    }
    std::cout << "\n";

    return 0;
}

// std::transform with std::mem_fn.
std::vector<Person*> getVector(const std::vector<std::shared_ptr<Person>>& people)
{
    Person* person;
    std::shared_ptr<Person> p;
    std::vector<Person*> result;

    std::transform(people.begin(), people.end(), std::back_inserter(result), 
        std::bind1st(std::mem_fn(&Person::getPerson), &p));

    return result;
}

Here are the instructions for the g++ compiler:

g++ -std=c++11 -Wall -Wextra main.cpp -o main
Ðаn
  • 10,934
  • 11
  • 59
  • 95
Rainer H.
  • 85
  • 2
  • 7
  • 1
    `std::move` on a pointer (as in `std::move(p)` in `getPerson`) is pointless. Just return the pointer. – Pete Becker Jan 06 '20 at 19:48
  • 1
    @PeteBecker It's *worse* than pointless, it may pessimise RVO done by the compiler. – Jesper Juhl Jan 06 '20 at 19:51
  • [Related](https://stackoverflow.com/questions/7822652/using-bind1st-for-a-method-that-takes-argument-by-reference). If you want `bind1st` to work, change signature of `getPerson` as taking `shared_ptr` with no reference. Now your problem is that you have two the same overloads for `operator()(const shared_ptr&)` inside `binder1st`. [Working bind1st](https://godbolt.org/z/w28rLm). – rafix07 Jan 06 '20 at 22:13

1 Answers1

1

Just use a lambda. It makes the code a lot easier to understand and you can get rid of the GetPerson function. Doing so would make your transform call look like

std::transform(people.begin(), 
               people.end(), 
               std::back_inserter(result), 
               [](const auto& elem){ return elem.get(); });
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • 1
    is there a significant difference between `auto elem` and `const auto& elem`? – Quest Jan 06 '20 at 19:39
  • 3
    @Quest In this case, yes. Just using `auto` would make a copy and that is unnessacary and because `shared_ptr` is atomically reference counted very expensive. By taking by reference we mitigate all of the cost. If it was something like an `int`, I would just take by value as long as I didn't need to update its value. – NathanOliver Jan 06 '20 at 19:41
  • 1
    "is there a significant difference between auto elem and const auto& elem?" - yes. One makes a copy (of a potentially expensive to copy object), the other does not. – Jesper Juhl Jan 06 '20 at 19:48
  • @NathanOliver Many thanks for your response. The source code is only a minimal example, so the irrelevant parts were removed. I already had implemented the ```getVector()``` function in two ways: lambda expression (working) and with ```std::transform``` and ```std::mem_fn``` (not working). I would like to know how to implement the function with ```std::transform``` and ```std::mem_fn``` so that I can compare the two functions (readability and programming effort). – Rainer H. Jan 06 '20 at 20:04
  • @RainerH. There really isn't a good way to do that with `mem_fn`. Since `GetPerson` is a non static member function you need to have a `Person` instance it uses to call the function on. You'd have to bind a `Person` to the return of `mem_fn` and add a `placeholder` for the shared pointer that transform is going to pass to it. That would look like `Person p; std::transform(..., std::bind(std::mem_fn(&Person::getPerson), p, _1));` but at that point you can just use bind directly. FWIW, just use a lambda. It's the standard convection and most everyone understands them. – NathanOliver Jan 06 '20 at 20:19
  • @NathanOliver Thank you very much for your help, explanations and the advice to use lambdas! – Rainer H. Jan 06 '20 at 21:09