Search code examples
c++pointersreferencecopy-constructormove-constructor

C++ - Updating value in pointer gets overridden


I've recently begun learning C++ and I'm having some trouble updating a pointer in my Movie class. I've got a feeling I have an issue somewhere in my Move/Copy constructors but have been trying to solve this issue for hours, swapping pointers to references to values and finally coming here for help.

I have a class Movie, which contains a string name, string rating and int watched member variables. Another class Movies, holds a vector of the movies. On movie, my method increment_watched is supposed to increment the int watched by one, the value does get incremented but it seems like the value is not saved. The display_all_movies method in Movies, which simply displays the movies that it stores holds the old value for watched. I know the issue is probably something really small but I haven't been able to work out why the value isn't being saved.

If someone could explain why the pointer value seems to be getting overridden I'd appreciate it greatly. Thanks in advance!

Code is below, there is some debug couts in there.

Movie.h

#ifndef _MOVIE_H_
#define _MOVIE_H_
#include <string>
#include <iostream>

class Movie {
private:
    std::string *name;
    std::string *rating;
    int *watched;
public:
    std::string get_name();
    std::string get_rating();
    int get_watched();
    void increment_watched();
    Movie(std::string name, std::string rating, int watched_val); // normal constructor
    Movie(const Movie &source); // copy constructor
    Movie(Movie &&source); // move constructor
    ~Movie();

};

#endif // _MOVIE_H_

Movie.cpp

#include "Movie.h"

std::string Movie::get_name() {
    return *name;
}

std::string Movie::get_rating() {
    return *rating;
}

int Movie::get_watched() {
    return *watched;
}

void Movie::increment_watched() {
    std::cout << *watched << std::endl;
    (*watched)++; // TODO FIX!
    std::cout << *watched << std::endl;
}

Movie::Movie(std::string name, std::string rating, int watched_val) 
    : name{nullptr}, rating{nullptr}, watched{nullptr}{   
    std::cout << "Creating movie " << watched_val << " with normal constructor" << std::endl; 
    this->name = new std::string{name};
    this->rating = new std::string{rating};
    this->watched = new int{watched_val};

}

Movie::Movie(const Movie &source) 
    : Movie(*source.name, *source.rating, *source.watched) {
        std::cout << "Creating movie " << *source.watched << " with copy constructor" << std::endl; 

}

Movie::Movie(Movie &&source)
    : Movie(*source.name, *source.rating, *source.watched) {
        std::cout << "Creating movie " << *source.watched << " with move constructor" << std::endl; 
        source.name = nullptr;
        source.rating = nullptr;
        source.watched = nullptr;
}

Movie::~Movie() {
        delete name;
        delete rating;
        delete watched;
}

Movies.h

#ifndef _MOVIES_H_
#define _MOVIES_H_
#include <vector>
#include <string>
#include <iostream>
#include "Movie.h"

class Movies {
private:
    static int count;
    std::vector<Movie> *movies;
public:
    void add_movie(std::string &&name, std::string &&rating, int &&watch);
    void increment_movie_count(std::string &&name);
    void display_all_movies();
    void count_movies();
    Movies();
    Movies(const Movie &source); // copy constructor
    Movies(Movie &&source); // move constructor
    ~Movies();

};

#endif // _MOVIES_H_

Movies.cpp

#include "Movies.h"

int Movies::count = 0;

void Movies::add_movie(std::string &&name, std::string &&rating, int &&watch) {
    bool contains {false};

    for(auto movie : *movies) {
        if(movie.get_name() == name) {
            contains = true;
        }
    }
    if(!contains) {
        movies->push_back(Movie{name, rating, watch});
        count++;
    }
}

void Movies::increment_movie_count(std::string &&name) {
    for(auto movie : *movies) {
        if(movie.get_name() == name) {
            movie.increment_watched();
        }
    }

}

void Movies::display_all_movies() {
    for(auto movie : *movies) {
        std::cout << "Title " << movie.get_name() << " Rating " << movie.get_rating() << " Watched " << movie.get_watched() << std::endl;
    }
}

void Movies::count_movies() {
    std::cout << "There are " << count << " movies " << std::endl; 
}

Movies::Movies() {
    movies = new std::vector<Movie>{};
}

Movies::~Movies() {
    delete movies;
}

And finally main.cpp

#include <iostream>
#include "Movies.h"

using std::cout;
using std::endl;

int main() {

    Movies *my_movies;


    my_movies = new Movies();

    (*my_movies).count_movies();

    my_movies->add_movie("Big", "PG-13", 2);
    my_movies->increment_movie_count("Big");

    my_movies->display_all_movies();

    return 0;
}

Solution

  • Changefor(auto movie : *movies) to for(auto& movie : *movies) to update the original movie objects. Otherwise, you're only updating copies of the movie objects.