-2
\$\begingroup\$

I am not sure whether it is the right way or am I deleting the memory properly. I have seen here some custom implementation of linked list, I want to do it simple way. Can anyone please suggest is it right or not? Should I use shared ptr, even if I do it, how to implement it correctly.

.hpp

class Camera {
    Camera()
    {}
    ~Camera()
    {}

}; typedef struct Node *NodePtr;

// Now declare the structure type itself
struct Node
{
    Camera_ x;
    NodePtr next;
};

.cpp

NodePtr head = NULL;
NodePtr* ptr = &head;
for (int n = 0; n<n1; ++n)
{
    *ptr = new Node;
    (*ptr)->x = Camera(); //storing camera objects
    ptr = &(*ptr)->next;
}

// terminate the list.
*ptr = NULL;

// walk the list,
for (NodePtr cp = head; cp->next != NULL; cp = cp->next)
{
    // doing camera functions

}
\$\endgroup\$

closed as off-topic by ratchet freak, vnp, Marc-Andre, forsvarir, t3chb0t Feb 7 '17 at 21:43

This question appears to be off-topic. The users who voted to close gave these specific reasons:

  • "Questions must involve real code that you own or maintain. Pseudocode, hypothetical code, or stub code should be replaced by a concrete implementation. Questions seeking an explanation of someone else's code are also off-topic." – forsvarir, t3chb0t
  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – ratchet freak, vnp, Marc-Andre
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Are the cameras all identical objects? It's not clear what code is to be reviewed, as it looks like you have presented a sanitized version of your real code. \$\endgroup\$ – 200_success Feb 7 '17 at 17:27
  • \$\begingroup\$ I want to know is it the right way? No , there would be different camera objects. \$\endgroup\$ – Angelina Feb 7 '17 at 17:43
  • \$\begingroup\$ why not std::list? \$\endgroup\$ – pm100 Feb 7 '17 at 17:54
  • \$\begingroup\$ typedefs like NodePtr are an abomination: They just add more mental overhead. \$\endgroup\$ – Deduplicator Feb 7 '17 at 18:55
  • \$\begingroup\$ Check for cp != NULL instead of cp->next != NULL. \$\endgroup\$ – G. Sliepen Feb 7 '17 at 20:15
2
\$\begingroup\$

if you dont want to use the c++ standard data structures then here are a few comments

for (int n = 0; n<n1; ++n)
{
    *ptr = new Node;
    (*ptr)->x = Camera(); //storing camera objects
    ptr = &(*ptr)->next;
}

First, you are storing local (stack) camera objects in your linked list. This is asking for trouble if you try to do more with the list than use it all in one function. The stack memory will be released on function exit

You never delete the Node objects that you create. This is bad. You need a Node destructor plus you should encapsulate the list in a class that has a destructor too.

Or just use std::list

You also refer to a type Camera_, this is not defined - I assume this is a typo

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.