-2

Recently I've started working on my first OOP project and after having written the program I am trying to optimize it for code efficiency. I want to place the parts of the program that are being copied a lot on the heap.

I can't understand why in certain places objects are copied. An example:

In the main.cpp movies object, which stories movie objects, is created. Add_movie function is called that checks if the movie we are trying to add has already been added, if not, we create a temp object, initialize its private members to the argument values being passed, append it to the vector of the movies object. A copy constructor would be called when movie object is being appended to the vector. WHY? I can't understand the part WHY is it being copied? Is it because of the scope???

If there was an object initialized in the main like

Movie movie1{arguments};

and other movie is created based on movie1

Movie movie2{movie1}.

It makes sense to me, but in the example I gave, it doesn't make sense to me at all

The example of the function I am referring to

bool Movies::add_movie(std::string name, std::string rating, int watched)
{
    for (const Movie& obj : movies_list)
    {
        if (obj.get_name() == name) // search for a match
        {
            return false; // if found stop executing
        }
    }
    Movie temp{ name, rating, watched }; // creates a new object and initializes its private members to the passed arguments
# movies_list.push_back(temp); // appends the object to the vector
# ***    return true;
}

I don't know if it will help, but there is the code of the program

**main.cpp **

#include "Movie.h"
#include "Movies.h"

#include <iostream>
#include <string>

void add_movie(Movies& obj, std::string name, std::string rating, int watched)
{
    if (obj.add_movie(name, rating, watched))
    {
        std::cout << name << " succesfully added" << std::endl;
    }
    else
    {
        std::cout << name << " already has been added" << std::endl;
    }
}
// if the parent increment_watched function returns true, inform the user about the result of the operation
void increment_watched(Movies &obj, std::string name)
{
    if (obj.increment_watched(name)) // if Movies::increment_watched returns
    {
        std::cout << name << " watch count succesfully incremented by 1" << std::endl;
    }
    else {
        std::cout << name << " movie not found" << std::endl;
    }
}

int main()
{


    Movies list;

    add_movie(list, "Fight Club", "A", 1);
    add_movie(list, "Fight Club", "A", 1);
    add_movie(list, "Inception", "A", 1);

    increment_watched(list, "Fight Club");
    increment_watched(list, "Else Test");
    

    list.display();

    return 0;
}

movies.cpp

#include "Movie.h"
#include "Movies.h"

#include <iostream>

bool Movies::add_movie(std::string name, std::string rating, int watched)
{
    for (const Movie& obj : movies_list)
    {
        if (obj.get_name() == name) // search for a match
        {
            return false; // if found stop executing
        }
    }
    Movie temp{ name, rating, watched }; // creates a new object and initializes its private members to the passed arguments
    movies_list.push_back(temp); // appends the object to the vector
    return true;
}
void Movies::display() const
{
    if (movies_list.size() == 0) // checks the vector size
    {
        std::cout << "The list is empty" << std::endl;
    }
    else
    {
        std::cout << "\nThe list of the movies: " << std::endl;
        std::cout << "----------------------------" << std::endl;

        for (const Movie& obj : movies_list) 
        {
            obj.display_members(); // accesses the private members of the object that are stored in the vector and outputs them to the user
        }
    }
}
bool Movies::increment_watched(std::string name)
{
    for (Movie &obj : movies_list) // iterates through the movie objects until finds the match in name
    {
        if (obj.get_name() == name)
        {
            obj.increment_watched(); // increments watched by 1 
            return true;
        }
    }
    return false;
}

movie.cpp

#include <string>
#include <iostream>
#include "Movie.h"

// constructor for initializing private members of the object
Movie::Movie(std::string name, std::string rating, int watched)
{
    this->name = name;
    this->rating = rating;
    this->watched = watched;
}
// get methods
std::string Movie::get_name() const { return name; }
std::string Movie::get_rating() const { return rating; }
int Movie::get_watched() const { return watched; }

// display private members
void Movie::display_members() const
{
    std::cout << "Name: " << get_name() << std::endl;
    std::cout << "Rating: " << get_rating() << std::endl;
    std::cout << "Times watched: " << get_watched() << std::endl;
    std::cout << "\n" << std::endl;
}
// setter function
void Movie::increment_watched() {watched++;}

// DEBUGGING
Movie::Movie(const Movie &obj):name{obj.name}, rating{obj.rating}, watched{obj.watched} {std::cout << "copy constructor called for " << name << std::endl;}
Movie::~Movie() {std::cout << "destructor called for movie " << name << std::endl;}

Debugging the program for hours to see which parts are being copied, when copied, when destructed to get a better grasp.

Watching countless videos that explain the lifetime of the objects, copy constructors, destructors, but it still doesn't make sense for me!

trincot
  • 317,000
  • 35
  • 244
  • 286
  • 2
    Please post a [mre]. – wohlstad Jan 28 '23 at 09:48
  • Too long to fully read, but be aware for e.g. `add_movie(std::string name, ...)` you *copy* the string already! To avoid, accept by const reference instead: `add_movie(std::string const& name, ...)`! – Aconcagua Jan 28 '23 at 10:01
  • `movies_list.push_back(temp);` why wouldn't this copy? You have an object _outside_ of vector, now you need the objects with the same data _inside_ the vector. As you cannot have single object in two places at the same time, a copy is made. You can define move constructor to move all content instead and avoid additional allocations. – Revolver_Ocelot Jan 28 '23 at 10:04
  • It's the nature of objects getting copied anywhere you do not use references or pointers. Then the vector is yet a bit special: Unless a vector of pointers or references it holds its object by value in a specifically allocated part of memory. Now the problem is: If an object has local scope (i.e. resides in a piece of memory used for the stack), how does it get into the vector's internal buffer, which resides at a totally different memory location (on the heap). You cannot take scissors and cut your memory into pieces to move around, so all that remains is: *copying*... – Aconcagua Jan 28 '23 at 10:04
  • 1
    To quote two lines of your code `Movie temp{ name, rating, watched }; movies_list.push_back(temp); // appends the object to the vector` Your comment here is incorrect. `movies_list.push_back(temp)` constructs and appends a *copy* of `temp` into `movies_list`. That's the way the `push_back()` member function of `std::vector` is specified. – Peter Jan 28 '23 at 10:07
  • [...] You cannot simply do `memcpy`, though, this just too easily breaks code (like leading to double deletion of dynamically allocated objects), thus you need copy constructors(*) telling the compiler how to copy *correctly*. As the original object remains untouched you need to do a deep copy, though – which can get pretty expensive, which is why moving semantics have been introduced with C++11 which *move* the internal data to a new location (shallow copy + clearing original object). Again it is the constructor(*) telling the compiler how to do so correctly. *(\*) or assignment operator(s)* – Aconcagua Jan 28 '23 at 10:09

1 Answers1

2

push_back() takes an object and appends it at the end of the vector. It has to make a copy because it must keep the original object intact because you might need it later. If you want to avoid the copy, you’d have you use std::move to trigger the move constructor.

movies_list.push_back(std::move(temp));

However, in your example you basically want to construct an object at the end of the vector. emplace_back is just what you need; no copying or moving, just pass the constructor arguments.

movies_list.emplace_back(name, rating,watched);
guard3
  • 823
  • 4
  • 13
  • Relevant discussion of using `std::move()` with `std::vector::push_back()` at https://stackoverflow.com/questions/11572669/move-with-vectorpush-back In short: not necessarily beneficial – Peter Jan 28 '23 at 12:16