0

I've got a homework that requires me to have a repository of dogs. I have to have a dynamic array, templated. For some reason, I get a lot of memory leaks and I don't know where they are from. I've tried using Deleaker but it says "Unknown" at source file.

To be honest, I don't actually understand where I should deallocate memory since I can't use delete.

This is my dynamic array definition and declaration (not all of it, just until the destructor), if it helps.

template <typename T>
class DynamicVector
{
private:
    T* elems;
    int size;
    int capacity;

public:
    // default constructor for a DynamicVector
    DynamicVector(int capacity = 10);

    // copy constructor for a DynamicVector
    DynamicVector(const DynamicVector& v);
    ~DynamicVector();
    DynamicVector& operator=(const DynamicVector& v);
};
template <typename T>
DynamicVector<T>::DynamicVector(int capacity)
{
    this->size = 0;
    this->capacity = capacity;
    this->elems = new T[capacity];
}

template <typename T>
T& DynamicVector<T>::operator[](int index)
{
    return this->elems[index];
}

template <typename T>
DynamicVector<T>::DynamicVector(const DynamicVector<T>& v)
{
    this->size = v.size;
    this->capacity = v.capacity;
    this->elems = new T[this->capacity];
    for (int i = 0; i < this->size; i++)
        this->elems[i] = v.elems[i];
}

template <typename T>
DynamicVector<T>::~DynamicVector()
{
    delete[] this->elems;
}

template <typename T>
DynamicVector<T>& DynamicVector<T>::operator=(const DynamicVector<T>& v)
{
    if (this == &v)
        return *this;

    this->size = v.size;
    this->capacity = v.capacity;

    auto aux = new T[this->capacity];

    delete[] this->elems;

    this->elems = aux;
    for (int i = 0; i < this->size; i++)
        this->elems[i] = v.elems[i];

    return *this;
}

Should I define a destructor in Dog.h and Dog.cpp also? Or in my dog repository (that uses the dynamic vector)?

Edit with dog class and repository:

class Dog
{
private:
    std::string breed;
    std::string name;
    int age;
    std::string photograph;

public:
    // default constructor for a dog
    Dog();

    // constructor with parameters
    Dog(const std::string& breed, const std::string& name, const int& age, const std::string& photograph);
    // returns true if two dogs have the same name
    bool operator==(const Dog & d);
    //returns the breed
    std::string getBreed() const { return breed; }
    //returns the name
    std::string getName() const { return name; }
    //returns the age
    int getAge() const { return age; }
    //returns the photograph
    std::string getPhotograph() const { return photograph; }
    void setName(const std::string& n);
    void setAge(const int& a);
    void show();
};

class Repository
{
private:
    DynamicVector<Dog> dogs;
    int current;

public:
    /*
    Default constructor.
    Initializes an object of type repository.
    */
    Repository();

    /*
    Adds a dog to the repository.
    Input: d - Dog.
    Output: the dog is added to the repository.
    */
    void addDog(const Dog& d);

    //I have more methods here but they are irrelevant
}

As you can notice, I only have constructors in these classes so maybe that's the problem, considering the rule of three.

xskxzr
  • 12,442
  • 12
  • 37
  • 77
Bryuki HK
  • 41
  • 4
  • Are you using dynamical allocation to create your `DynamicVector` instances? Check if you have any `new DynamicVector<...>()` in your code. – Clearer Apr 05 '18 at 08:38
  • 3
    Please provide more information. The example actually looks correct to me. – informant09 Apr 05 '18 at 08:40
  • 1
    Do you have a *copy assignment operator*? – Galik Apr 05 '18 at 08:43
  • See [rule of three](https://stackoverflow.com/q/4172722/1312382)! – Aconcagua Apr 05 '18 at 08:45
  • @Galik yes, I do, I'll edit my post with it. – Bryuki HK Apr 05 '18 at 08:48
  • @informant09 My issue is finding the memory leaks. I have the above defined DynamicVector . I use it to define a repository of objects of type "Dog". I have no idea where memory leaks could come from. Should I have a destructor for the class Dog or the class Repository also? that's what I'm wondering – Bryuki HK Apr 05 '18 at 08:52
  • @Clearer I have this kind of code (that resembles what you said): DynamicVector v = repo.filterByBreedAndAge("Beagle", 6); – Bryuki HK Apr 05 '18 at 08:53
  • 3
    That seems fine to me. Only thing I would do differently is call `new` *before* changing the size and capacity of the vector in your *copy assignment* so you don't end up with an invalid object in the event of an exception. – Galik Apr 05 '18 at 08:55
  • is this a HW assignment, where you have to recreate `std::vector`? Else just use `std::vector`... – JHBonarius Apr 05 '18 at 08:55
  • If your Dog-class or repository allocates memory dynamically, then yes! Can you maybe post a summary of your dog class or your repository? – informant09 Apr 05 '18 at 09:01
  • 1
    @JHBonarius yes, it is a HW assignment and I am not allowed to use std::vector yet :( – Bryuki HK Apr 05 '18 at 09:04
  • 1
    @BryukiHK that's like "we're teaching to to ride a bicycle, but you are not allowed to use the pedals yet." ;) – JHBonarius Apr 05 '18 at 09:07
  • @informant09 yes, I'll edit my post! – Bryuki HK Apr 05 '18 at 09:10
  • 1
    @JHBonarius Or the handle or the saddle. – Clearer Apr 05 '18 at 09:11
  • 1
    The dog and the repository class are completely fine. I still need more information ;) You basically just need a destructor if you allocate memory on the heap. So if there are no pointers in your class at all (and no virtual methods), you do not need a destructor. – informant09 Apr 05 '18 at 09:18
  • Do you get a memory leak if you just add integers to it? – Galik Apr 05 '18 at 09:24
  • @JHBonarius that's funny – Bryuki HK Apr 05 '18 at 09:35
  • 1
    @Bryuki HK if you have a `DynamicVector` in global scope that could cause some problems. In general avoid global scope. – Raxvan Apr 05 '18 at 09:39
  • "*... says "Unknown" at source file*" suggests that you compiled without debugging symbols - try `g++ -g` or equivalent. Also, why are you using a raw pointer? `std::unique_ptr` and `std::vector` exist to do this kind of memory management for you. – Toby Speight Apr 05 '18 at 15:19

2 Answers2

1

Why don't you just use std::vector? Dynamic allocation is really tricky; you gotta follow the recommend but undocumented pattern for RAII with several requirements: copy/move constructor, assignment operator, swap function, destructor. You'd better stick to the std::vector, unless - of course - it is banned.

Red.Wave
  • 2,790
  • 11
  • 17
  • 1
    This is not an answer, but a comment. Your question was actually answered in the comments already: its banned because it's a homework assignment. (probably to recreate std::vector) – JHBonarius Apr 05 '18 at 20:39
0

So take the long way and define all special functions - and a swap function to be used in some of those in order to refactor the code. If either one of those functions is left default, sooner or later someway or other you will end up with dangling pointers, double deleting, use of deleted pointers and hopefully an early crash to finish the nightmare. All special members MUST be properly defined. The "big 5" (formerly "big 3") is the idiomatic way of implementing RAII. You gotta define destructor copy-constructor and copy-assignment(plus the move-counterparts). I wish you could at least use unique_ptr.

Red.Wave
  • 2,790
  • 11
  • 17