Skip to main content
Code Review

Return to Answer

added 506 characters in body
Source Link
Cris Luengo
  • 7k
  • 1
  • 14
  • 37

Just one quick comment to add to the previous answers. You have static variables thatlike this:

static bool* triActive = NULL;

These are all pointers, you dynamically allocate space forassigned the pointer to a single valuememory block, and store its pointer in your static variable. Theyou need an at-exit function needs to free this allocatedthese memory blocks. All of thatIn C++ it is considered good practice to store a scalaravoid "naked pointers" like these. If you follow that likely takes up less space thanadvice, you won’t need the pointer to itat-exit function any more. There is no

For example, you could instead do:

std::vector<bool> triActive;

Your initialize function then just needs to resize the array to the appropriate size:

if(triActive.empty())
 triActive.resize(numTris, false);

Note that you also don’t need for thisthe static keyword here. Just store those values directly as globalIn file scope, all variables are global, and will live for the duration of the program (since this is a DLL/SO, for the duration of it being loaded in memory). The initializestatic function must assign a valuehere means to them, then you can forget aboutnot make it visible outside the file.

Inside a function, the static keyword means to keep the variable alive in between function calls.

Just one quick comment to add to the previous answers. You have static variables that are all pointers, you dynamically allocate space for a single value, and store its pointer in your static variable. The at-exit function needs to free this allocated memory. All of that to store a scalar that likely takes up less space than the pointer to it. There is no need for this. Just store those values directly as global variables. The initialize function must assign a value to them, then you can forget about it.

Just one quick comment to add to the previous answers. You have static variables like this:

static bool* triActive = NULL;

These are assigned the pointer to a memory block, and you need an at-exit function to free these memory blocks. In C++ it is considered good practice to avoid "naked pointers" like these. If you follow that advice, you won’t need the at-exit function any more.

For example, you could instead do:

std::vector<bool> triActive;

Your initialize function then just needs to resize the array to the appropriate size:

if(triActive.empty())
 triActive.resize(numTris, false);

Note that you also don’t need the static keyword here. In file scope, all variables are global, and will live for the duration of the program (since this is a DLL/SO, for the duration of it being loaded in memory). static here means to not make it visible outside the file.

Inside a function, the static keyword means to keep the variable alive in between function calls.

Source Link
Cris Luengo
  • 7k
  • 1
  • 14
  • 37

Just one quick comment to add to the previous answers. You have static variables that are all pointers, you dynamically allocate space for a single value, and store its pointer in your static variable. The at-exit function needs to free this allocated memory. All of that to store a scalar that likely takes up less space than the pointer to it. There is no need for this. Just store those values directly as global variables. The initialize function must assign a value to them, then you can forget about it.

default

AltStyle によって変換されたページ (->オリジナル) /