C++: Is it ok to use shared pointers in a function as parameters or am I wasting memory?

Question

I've just started learning C++ a few weeks ago, so I'm not an expert on memory management (which is really important in this case, but I just haven't got my head around computer architecture and how pointers store information). That's why I'm not sure if using a shared_ptr as a function parameter is a waste of memory or not.

What I'm making is a program with two classes called Usuario and Product. The first class represents the User and has two private variables: std::string m_name (the name it is refered as) and float m_money (the quantity of money the User has), with their respective Getters and Setters.

The second class represents the products and has three private variables: std::string m_name (the name of the product), float m_price (its price), float m_iva (a tax called Impuesto sobre el Valor Agregado, which I set at 12%, so m_iva = m_price x 0.12) and float m_totalPrice (final price of the product, so m_totalPrice = m_price + m_iva), with their respective getters and two setters related to m_name and m_price.

Having set these classes with their respective .h and .cpp files, I created the objects in the main() function using std::shared_pointer<class> objectName = std::make_shared<class> (parameters) and pass them to other functions as parameters with std::shared_pointer<class> aDiferentName. Here is, for instance, a function which allows the user to put his name and amount of money:

void SetCurrentUser(std::shared_ptr<Usuario> defaultUser) //Not sure if this is efficient.
{
    std::string userName;          //Variable to which the user input will be stored
    float money;                   //Variable to which the user input will be stored

    std::cout << "Input your name:  "; std::cin >> userName; defaultUser->SetName(userName);  //Sets a new name to defaultUser
    std::cout << "Input the amount of money you have:   "; std::cin >> money; defaultUser->SetMoney(money); //Sets a new amount of money to defaultUser

}

And here is a function which prints the information of an item and tells the user whether they can buy it or not.

    void PrintCurrentStatus(std::shared_ptr<Product> AnyProduct, std::shared_ptr<Usuario> User) //Not sure if this is efficient either
{ 
        std::cout << "\n" << std::setprecision(3) << AnyProduct->GetName() << " + " << AnyProduct->GetPrice() << "$" 
                  << " + " << AnyProduct->GetIVA() << " IVA = " << AnyProduct->GetTotal() 
                  << "$" << std::endl;            //Prints the product information like this: *line* *m_name* + *m_iva*IVA = *m_totalPrice*

        if ( User->GetMoney() >= AnyProduct->GetTotal() ) { //If the user's amount money is equal to or greater than the total price of the product. 
            std::cout << "User is allowed to pay.";
        }
        else { //If the user's amount of money is less than the total price of the product.
            std::cout << "User doesn't have enough money."; 
        }
    }

And here is the main() function, where I created all the objects(including the User, which I set with " " and 0 as initial parameters):

int main()
{
    std::shared_ptr<Usuario> CurrentUser = std::make_shared<Usuario>(" ", 0);  //I created this object so that it can be set properly with the function SetCurrentUser
    SetCurrentUser(CurrentUser);   //Here the user sets it as they wish.
    std::shared_ptr<Product> Product1 = std::make_shared<Product>("Butter", 10.56, 10);    //Product1's name is Butter, costs 10.56 dollars and there are 10 units available
    std::shared_ptr<Product> Product2 = std::make_shared<Product>("Milk", 5.45, 10);   //Product2's name is "Milk", costs 5.45 dollars and there are 10 units of it available.
    PrintCurrentStatus(Product1, CurrentUser);  //Prints Product1's information and tells the user whether they can buy it or not.
    PrintCurrentStatus(Product2, CurrentUser); //Prints Product2's information and tells the user whether they can buy it or no.

    std::cin.ignore();       //We need to erase the contents of cin.
    std::cin.get();          //Waits until the user presses enter.
    return 0;                //The program ends here.
}

All these functions are located in the same .cpp file.

So, all things said, the program behaves exactly how I want it to behave. However, I do want to know: am I wasting memory by putting std::shared_ptr as function parameters corresponding to shared pointers to objects created inside the main function? Is there a more efficient way to get the same results?

Answer 1

Guideline from expert

Herb Sutter - one of C++ experts wrote whole article about shared_ptr and other smart pointers here: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/

This is what is applicable to your issue:

Guideline: Don’t pass a smart pointer as a function parameter unless you want to use or manipulate the smart pointer itself, such as to share or transfer ownership.

TL;DR; shared_ptr

Making long story short - shared_ptr stores much more than just pointer (reference count, weak count, deleter function), so if you do not use your function to store the shared_ptr somwhere in class then the pointer is unnecessary. Generally you are "paying" for what you are not using. Whould you do that in real life?

It can and probably will make performance worse. To what extent? It has to be measured. Generally when talking about performance we should not make any assumptions before measuring the performance.

How to fix it?

Pass by (potentially const) reference to type stored in shared_ptr or even by value if you do not need to manipulate smart pointer. Maybe pass by raw pointer if you need to pass nullable value (but I would recommend explicit notation about optional argument using std::optional or boost::optional).

Again citing Herb Sutter:

Guideline: Prefer passing objects by value, *, or &, not by smart pointer.

So I would change this:

void SetCurrentUser(std::shared_ptr<Usuario> defaultUser)

to this:

void SetCurrentUser(Usuario &defaultUser)

or this:

void SetCurrentUser(Usuario *defaultUser)

The first one logically implies that the function may take ownership of the smart pointer while the others only say that it will use the object of type Usuario.

Answer 2

Technically, you're "wasting memory" because a std::shared_ptr consists of two internal pointers (with typical C++ implementations), so passing a std::shared_ptr by value as a parameter to a function ends up actually passing two pointers; while passing a plain pointer argument, well, obviously passes one.

So what? The benefits of using smart pointers outweigh the slight overhead.

But, you can do even better than that. In nearly all use cases, you can pass parameters by reference, instead:

void SetCurrentUser(const std::shared_ptr<Usuario> &defaultUser) 

With typical C++ implementations, this ends up passing a single actual pointer, and "waste" as much stack as passing a plain pointer. You get all the benefits of using smart pointers, while being penalized the same amount of precious bytes on the stack. There are, of course, some fundamental differences between passing parameters by value, versus by reference. But with nearly all simple, basic programs of the kind you're writing now, there is no functional difference, and your C++ book should fully explain to you what they are, when it does begin to matter.

Typically, there will probably be a little bit of additional runtime overhead, coming from an indirect reference, but that's most likely will get offset by saving a few CPU cycles by not having to update the internal reference count held in the smart pointer; and, in any case, it wouldn't be anything that anyone would notice, on modern multi-ghz platforms, unless one uses an atomic clock, or something.