1

I am building a class that that allocates a dynamic array of structs and the array of structs contains another struct. The thing is all the elements in the struct are of fixed size so would the conventional way of releasing the memory suffice (delete [ ] array)?

struct TRANSITION
{
    int transition;
    int next_state;
};

struct state
{
    int transitionCount = 0;
    string stateName;
    string stateAction;
    TRANSITION transitions[50];
};

class constructor/destructor:

FSM(int n)
{
    numberOfStates = n;
    states = new state[numberOfStates];
    currentState = 0; //First state numbered state 0
    stateCount = 0;
}
~FSM() { delete[]states; };

Is this the correct way to delete this array?

5
  • 2
    What type is states? And why not just use std::vector or something else that does this for you? Commented Mar 27, 2016 at 20:12
  • I don't see static here. Commented Mar 27, 2016 at 20:14
  • "all the elements in the struct are static" (1) There are no static members in this code.(2) Use std::vector. Commented Mar 27, 2016 at 20:15
  • What I meant by static is that none of the elements within the struct are allocated with 'new'. So I do not have to delete the elements with 'delete [ ] states[0]->stateName' for example. But yes, the array is dynamic. Commented Mar 27, 2016 at 20:32
  • "static" has a very specific meaning in C++, and that's not it. Commented Mar 27, 2016 at 22:44

3 Answers 3

2

Short description: Yes, considering the code above, it is the correct way.

Long description:
Assuming that the variable states is a state*, the problem can be broken into two parts:

  1. Destruction of each of the elements of the array, and
  2. Destruction of the array itself

Since there is no explicit destructor defined for the struct, and the struct is being used in a new[] definition, a default implicit destructor is created by the compiler (Ref: ISO C++ standard working draft, page 287) for the struct. This destructor calls destructors of all the elements included in the struct and hence allows deletion of elements of the array one by one.

There is also an implicit operator delete[] that calls the individual destructors of all the array elements ensuring a proper cascade. (Ref:C++ Reference)

Overall, the combination allows deleting the array of structs using delete[].

Sign up to request clarification or add additional context in comments.

Comments

1
  1. None of the members are static.
  2. Yea it's pretty much the correct way to delete that array.

Comments

0

Yes, for each new there is a delete. And given that we're dealing with a fixed size array the delete[] is exactly what is required.

Also good to have the new in the constructor and the delete in the destructor as per RAII principles.

Another way to verify could be to check for memory leaks in the program. In Visual Studio for example the following shows no memory leaks:

#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>

#include <iostream>
#include <string>

#ifdef _DEBUG
#ifndef DBG_NEW
#define DBG_NEW new ( _NORMAL_BLOCK , __FILE__ , __LINE__ )
#define new DBG_NEW
#endif
#endif  // _DEBUG

struct TRANSITION {
    int transition;
    int next_state;
};

struct state {
    int transitionCount = 0;
    std::string stateName;
    std::string stateAction;
    TRANSITION transitions[50];
};

class FSM {
public:
    FSM(int n);
    ~FSM();

    int numberOfStates;
    state* states;
    int currentState;
    int stateCount;
};

FSM::FSM(int n)
{
    numberOfStates = n;
    states = new state[numberOfStates];
    currentState = 0; //First state numbered state 0
    stateCount = 0;
}

FSM::~FSM() { delete[] states; };
//FSM::~FSM() {  };

int main()
{
    _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
    FSM fsm(3);
    return 0;
}

But if you remove the delete[] states; statement then it produces:

Detected memory leaks!
Dumping objects ->
{160} normal block at 0x0000024535F03B00, 16 bytes long.
 Data: < V 5E           > D8 56 F0 35 45 02 00 00 00 00 00 00 00 00 00 00 
{159} normal block at 0x0000024535F04730, 16 bytes long.
 Data: < V 5E           > B0 56 F0 35 45 02 00 00 00 00 00 00 00 00 00 00 
{158} normal block at 0x0000024535F03A10, 16 bytes long.
 Data: < T 5E           > F0 54 F0 35 45 02 00 00 00 00 00 00 00 00 00 00 
{157} normal block at 0x0000024535F03AB0, 16 bytes long.
 Data: < T 5E           > C8 54 F0 35 45 02 00 00 00 00 00 00 00 00 00 00 
{156} normal block at 0x0000024535F03DD0, 16 bytes long.
 Data: < S 5E           > 08 53 F0 35 45 02 00 00 00 00 00 00 00 00 00 00 
{155} normal block at 0x0000024535F046E0, 16 bytes long.
 Data: < R 5E           > E0 52 F0 35 45 02 00 00 00 00 00 00 00 00 00 00 
c:\users\username\documents\visual studio 2015\projects\project60\project60\main.cpp(41) : {154} normal block at 0x0000024535F052D0, 1472 bytes long.
 Data: <                > 03 00 00 00 00 00 00 00 00 00 00 00 CD CD CD CD 
Object dump complete.

It can be tricky to keep track of all the new and delete statements, so the best approach would normally be to use one of the standard library containers such as the prototypical std::vector.

Another, perhaps lighter, modification could be to use smart pointers such as std::unique_ptr:

#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>

#include <iostream>
#include <string>
#include <memory>

#ifdef _DEBUG
#ifndef DBG_NEW
#define DBG_NEW new ( _NORMAL_BLOCK , __FILE__ , __LINE__ )
#define new DBG_NEW
#endif
#endif  // _DEBUG

struct TRANSITION {
    int transition;
    int next_state;
};

struct state {
    int transitionCount = 0;
    std::string stateName;
    std::string stateAction;
    TRANSITION transitions[50];
};

class FSM {
public:
    FSM(int n);
    ~FSM();

    int numberOfStates;
    //state* states;
    std::unique_ptr<state[]> states;
    int currentState;
    int stateCount;
};

FSM::FSM(int n)
{
    numberOfStates = n;
    //states = new state[numberOfStates];
    states = std::make_unique<state[]>(numberOfStates);
    currentState = 0; //First state numbered state 0
    stateCount = 0;
}

//FSM::~FSM() { delete[] states; };
FSM::~FSM() {  };

int main()
{
    _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
    FSM fsm(3);
    return 0;
}

No risk of forgetting a delete and we can feel perhaps a bit safer not seeing any new statements either.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.