-1

So what I am trying to do, I have two classes named ClassA and ClassB, in ClassB there exists a pointer which is referring to an array of ClassA objects. But when I am trying to save the objects inside the array the details are not getting reflected. I tried debugging the same and it's giving me segmentation fault. Is there anything I am doing wrong?

Classes File:

#include <bits/stdc++.h>
using namespace std;

class ClassA{
    private:
        string name;
        string phone;
    public:
        ClassA(string name, string phone)
        {
            this->name = name;
            this->phone = phone;
        }
        string getName()
        {
            return name;
        }
        string getPhone()
        {
            return phone;
        }
};

class ClassB{
    private:
        ClassA* details;
        int size;
    public:
        ClassB()
        {
            size = 0;
            details = (ClassA*)malloc(5*sizeof(ClassA));
        }
        void addDetails(string name, string phone)
        {
            ClassA* temp = new ClassA(name, phone);
            details[size++] = *temp;
            // cout<<details[size-1].getName()<<" "<<details[size-1].getPhone()<<endl;
        }
        void print()
        {
            for(int i = 0; i < size; i++)
            {
                cout << details[i].getName() << " " << details[i].getPhone() << endl;
            }
        }
};

Driver Code:

#include "temp.h"
using namespace std;

int main()
{
    ClassB b;
    b.addDetails("A", "123");
    b.addDetails("B", "456");
    b.addDetails("C", "789");
    b.print();
    return 0;
}

Anyone, please help me out on this.

  • 1
    Don't mix malloc and new. In C++, use new keyword. Also, you can avoid using new/delete by using smart pointers. – kiner_shah Feb 24 '22 at 09:19
  • Your constructor of `classB` sets the `size` variable to 0. The body of the loop in the `print` method never gets executed. – typewriter Feb 24 '22 at 09:19
  • 2
    Also, in your addDetails(), you are doing something weird - `details[size++]`. `details` is a pointer to ClassA, not an array. – kiner_shah Feb 24 '22 at 09:21
  • 1
    Shouldn't this be something like `details = (ClassA*)malloc(size*sizeof(ClassA));`? – typewriter Feb 24 '22 at 09:22
  • 2
    `#include using namespace std;`? So called coding competition sites are _not_ learning resources. They often use bad practices and a dangerous mix of C and C++ without actually explaining anything correctly. Better a get a [good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and learn the basics properly. Modern C++ is soooo much easier. – Lukas-T Feb 24 '22 at 09:23
  • Yes, there was a mistake. I just updated the code. Thank you for pointing out. – Anurag Srivastava Feb 24 '22 at 09:23
  • I don't think the problem lies in the header file @churill – Anurag Srivastava Feb 24 '22 at 09:27
  • @Darth-CodeX sorry but your's was not the answer I was looking for. I want the same functionality with arrays only. But thank you so much for the tips. – Anurag Srivastava Feb 24 '22 at 12:41
  • @AnuragSrivastava If you find my answer helpful I have updated my answer to fit your criteria, then accept it – Darth-CodeX Feb 25 '22 at 02:41

2 Answers2

3

When you are calling malloc you won't call the constructor, so ClassA is not constructed. Plus, your malloc put only allocate a place for one object. You better use std::vector, see the example below.

#include <iostream>
#include <vector>
using namespace std;

class ClassA{
private:
    string name;
    string phone;
public:
    ClassA(string name, string phone)
    {
        this->name = name;
        this->phone = phone;
    }
    string getName()
    {
        return name;
    }
    string getPhone()
    {
        return phone;
    }
};
class ClassB{
private:
    std::vector<ClassA> details;

public:
   
    void addDetails(string name, string phone)
    {
        details.emplace_back(name, phone);
        //cout<<details[details.size()-1].getName()<<" "<<details[details.size()-1].getPhone()<<endl;
    }
    void print()
    {
        for(int i = 0; i < details.size(); i++)
        {
            cout << details[i].getName() << " " << details[i].getPhone() << endl;
        }
    }
};

int main()
{
    ClassB b;
    b.addDetails("A", "123");
    b.addDetails("B", "456");
    b.addDetails("C", "789");
    b.print();
    return 0;
}   
SHR
  • 7,940
  • 9
  • 38
  • 57
1

You are mixing malloc and new, which causes undefined behavior.

The C library function malloc() does not initialize the memory nor call the initializer of the class, since C does not have classes. That's why you should never call malloc() to allocate heap memory for classes/structs in C++. Whereas new is a operator which returns the exact data type and calls the initializer of the class\struct.

Some Problems:

  • Use size_t to store length of the array.
  • Use capacity to reduce the amount of call to re-allocate memory
  • If parameter is not going to change inside the function, write it as const std::string &
  • do not include <bits/stdc++.h>, instead include only those which are required
  • do not write this using namespace std;, read this
  • not creating a default constructor for ClassA to call new operator.
  • not creating a destructor to avoid memory leak

So, you final code should be TRY IT ONLINE:

#include <iostream>

class ClassA
{
private:
    std::string name;
    std::string phone;

public:
    ClassA() = default;
    ClassA(const std::string &name, const std::string &phone)
    {
        this->name = name;
        this->phone = phone;
    }
    std::string getName() const
    {
        return this->name;
    }
    std::string getPhone() const
    {
        return this->phone;
    }
};

class ClassB
{
private:
    ClassA *data;
    std::size_t len, cap;
    void resize()
    {
        std::size_t new_cap = this->cap * 2;
        ClassA *temp = new ClassA[new_cap];
        for (std::size_t i = 0; i < this->len; i++)
        {
            temp[i] = this->data[i];
        }
        this->cap = new_cap;
        this->data = temp;
    }

public:
    ClassB()
    {
        this->len = 0;
        this->cap = 5;
        this->data = new ClassA[this->cap];
    }
    void addDetails(const std::string &name, const std::string &phone)
    {
        if (this->len == this->cap)
            this->resize();
        this->data[this->len] = ClassA(name, phone);
        this->len++;
    }
    void print() const
    {
        for (std::size_t i = 0; i < this->len; i++)
        {
            std::cout << data[i].getName() << " " << data[i].getPhone() << "\n";
        }
        std::cout << std::endl;
    }
    std::size_t length() const
    {
        return this->len;
    }
    std::size_t capacity() const
    {
        return this->cap;
    }
    ~ClassB()
    {
        delete[] this->data;
    }
};

int main(void)
{
    ClassB b;
    b.addDetails("A", "123");
    b.addDetails("B", "456");
    b.addDetails("C", "789");
    b.addDetails("D", "123");
    b.addDetails("E", "890");
    b.addDetails("F", "567");
    b.addDetails("G", "753");
    b.print();
    std::cout << "Length of b = " << b.length() << "\n";
    std::cout << "Capacity of b = " << b.capacity() << std::endl;
    return 0;
}
Darth-CodeX
  • 2,166
  • 1
  • 6
  • 23