How to correctly read inputs into a dynamically allocated array of objects

Question

I am currently working on a project which is a ticket storage system that stores each ticket holder as an object.

To input the values, I am using a parameterized constructor. In the main function I have declared a dynamic memory block for an array of these objects.

The main issue I face is that in the for loop that initializes each object, the loop only runs once, then terminates. Here is the code:

#include <iostream>
#include <stdlib.h>
#include <string>

using namespace std;

class node
{
    string holder_name;
    int age;

public:
    node(string a, int b)
    {
        holder_name = a;
        age = b;
    }

    void view()
    {
        cout << "Name: " << holder_name << endl;
        cout << "Age: " << age << endl;
    }
};

int main()
{
    int count, i;
    cout << "Enter no of nodes" << endl;
    cin >> count;
    node *arr = (node *)malloc(sizeof(node) * count);

    for (i = 0; i < count; i++)
    {
        int b;
        string str;
        cout << "Enter name" << endl;
        cin >> str;
        cout << "Enter age" << endl;
        cin >> b;
        arr[i] = node(str, b);
        arr[i].view();
    }
    return 0;
}

The `for` loop depends on user input. For a better [mre], remove that factor. (If I choose to enter `1`, then the loop *should* run only once.) Instead of `int count; cin >> count;` hardcode a value, as in `int count = 2;`. You could also remove the user input from the loop, perhaps have everyone's name be `"Someone"` and their age be `i`. — JaMiT, Mar 13 '21 at 16:39

score 2 · Answer 1 · answered Mar 13 '21 at 16:34

2

Use Constructor and Destructor in your class.

    class node{
string holder_name;
int age;
public:
node(){
    
}
node(string a, int b)
{
    holder_name=a;
    age=b;
}

void view()
{
    cout<<"Name: "<<holder_name<<endl;
    cout<<"Age: "<<age<<endl;
}
~node()
{
    
}

Also don't use malloc for allocation in c++ instead use

    node*  arr= new node[count];

answered Mar 13 '21 at 16:34

Tahmid108

45
4

In this case, the destructor does not need to be explicitly defined. The compiler-generated destructor suffices ([rule of zero](https://stackoverflow.com/questions/44997955/rule-of-zero-confusion)). – JaMiT Mar 13 '21 at 17:29

score 1 · Answer 2 · answered Mar 13 '21 at 16:30

The assignment arr[i]=node(str,b) requires valid (constructed) values on the left side. malloc allocates the array memory but do not initialize array elements. C++ new operator is for such purposes.

node *arr= new node[count];

Just to compare the expressions

node *arr= (node*)malloc(sizeof(node)*count);
node *arr= new node[count];

The second expression is shorter then C malloc used, safer and automates a lot of work. But even more better if you don't deal with manual memory management.

auto arr= std::vector<node>(count);

anastaciu · Accepted Answer · 2021-03-13T18:26:44.423

That is a fine example why malloc is not used in C++, when you allocate memory for your nodes, the elements are not initialized.

Allocation in C++ is done with new wich guards against the aforementioned problem:

node *arr= new node[count];

In this case you'll also need to add a default constructor to your class

node() = default; or node(){};

A better solution, however, is to use one of the resizable containers provided by C++ containers library, for instance std::vector:

#include <vector>

int main()
{
    int count, i;
    cout << "Enter no of nodes" << endl;
    cin >> count;
    std::vector<node> arr; // now you have a resizable container
    for (i = 0; i < count; i++)
    {
        int b;
        string str;
        cout << "Enter name" << endl;
        cin >> str;
        cout << "Enter age" << endl;
        cin >> b;
        arr.push_back(node(str, b)); // keep adding elements
        arr[i].view();
    }
}

Here you could also create the vector with an initial size as suggested in S.M. answer, it really deppends on the sizes you expect for your container, if it's on the larger side it's better because it avoids memory reallocation, in which case the insersion would be exactly as you have it in your original code.

Footnote

I should add that using namespace std; is not the most recommended techinque, you can find the reasoning and alternative methods here:

Why is "using namespace std;" considered bad practice?

score -1 · Answer 4 · answered Mar 13 '21 at 17:44

-1

Edit as follows (with the fewest changes to your code).

#include <iostream>
#include <stdlib.h>
#include <string>

using namespace std;
class node
{
    string* holder_name;
    int age;
public:
    node(string a, int b)
    {
        holder_name = new string(a);
        age = b;
    }
    ~node()
    {
        Dispose();
    }

    void Dispose()
    {
        if (holder_name)
        {
            delete holder_name;
            holder_name = NULL;
        }
    }

    void view()
    {
        cout << "Name: " << *holder_name << endl;
        cout << "Age: " << age << endl;
    }

    // copy assignment
    void operator = (const node& D) {
        if (holder_name)
            delete holder_name;
        holder_name = new string( *D.holder_name );
        age = D.age;
    }
};

int main()
{
    int count, i;
    cout << "Enter no of nodes" << endl;
    cin >> count;
    node* arr = (node*)malloc(sizeof(node) * count);
    memset(arr, 0, sizeof(node) * count);
    

    for (i = 0; i < count; i++)
    {
        int b;
        string str;
        cout << "Enter name" << endl;
        cin >> str;
        cout << "Enter age" << endl;
        cin >> b;
        arr[i] = node(str, b);
        arr[i].view();
    }

    
    // avoid memory leak
    for (i = 0; i < count; i++)
        arr[i].Dispose();
    free(arr);

    return 0;
}

answered Mar 13 '21 at 17:44

HSh

29
3

Why does this help? Code-only answers tend to be cryptic, leading to [cargo cult programming](https://en.wikipedia.org/wiki/Cargo_cult_programming) instead of education. Try writing out a description of what you changed and why, relegating the actual code to merely an illustration of the answer, not the answer itself. – JaMiT Mar 13 '21 at 17:48
Do you think there were no educational points in my amendment? For example operator overloading. Thank you for your attention @JaMiT – HSh Mar 14 '21 at 10:57
By the way, the code that is the answer was modified (with additional description) after my post. Before that, there were secret points (like default constructor) that made me send this answer. Please note the correction time of the answer after my post. Of course, the solution is better with an explanation. Hoping to get better. @JaMiT – HSh Mar 14 '21 at 11:50
*"Do you think there were no educational points in my amendment?"* That is the gist of what I was getting at, yes. Without an explanation, there will likely be no education. *(For what it's worth: the downvote is not from me. Someone else apparently also disapproves of this answer, but declined to explain why.)* – JaMiT Mar 15 '21 at 00:32
Hmm... maybe I should read that question as *"Do you think there were no **potentially** educational points in my amendment?"* For this modified question, my answer is that I don't know. I can't tell what your amendment is by reading your answer, and I am not motivated enough to compare your code to the question's to see what you changed. Sorry. – JaMiT Mar 15 '21 at 00:59
@JaMiT Maybe most time I learned programming by code due to lack of educational resources. Anyway, I am glad that there was no negative vote from you. Thanks for the constructive conversation with you. Regards – HSh Mar 15 '21 at 11:38

How to correctly read inputs into a dynamically allocated array of objects

4 Answers4

Footnote