General C++ class questions

Started by
12 comments, last by frob 6 years, 6 months ago

I just have some general questions when it comes to C++ mainly focused on classes

1. When it comes to instantiating a class is there something like a static initializer? Or would it be better to have a static variable(s) and a corresponding flag to say whether or not the variables are ready to use?

2. Is it "bad" to throw exceptions in the constructor or should I use a 2 step initialization process? Example: I have TruckModel class and when I create a new instance I load in the mesh that this truck model uses. I throw an exception if the file cannot be found because I need the truck mesh in order to continue. This all happens within the TruckModel constructor

3. I am currently throwing some exceptions when critical errors happen and I guess exceptions do not work in the way I would expect. I sort of expect the program to come to screeching hault and fully stop execution, but it continues on if you just say to "continue" in the debugger. Simplified example:


#include <iostream>

int main()
{
	int count = 0;
	while (true)
	{
      		//Exception is thrown and there is no attempt to catch it
        	//Debugger says the exception occurs and complains, but I can carry on with my program like normal if I just ignore the complaints and keep stepping through until I come back to this point
		if (count == 10)
			throw std::exception("Something went wrong...");
      
		std::cout << "Count:" << count << std::endl;
		++count;
	}


	std::cout << "COMPLETE!" << std::endl;
}

4. Is it possible to hide a destructor from being directly called? I have classes that have a Release method to clean up various pointer resources used by the class instance. The destructor of the class does not call my Release method as that would free the resources prematurely in some cases. Example:


ResourceItem *ResourceCreator::createResourceItem(UINT size)
{
	ResourceItem resourceItem;
	system->CreateResourceItem(size, &resourceItem.dataPointer);
	resourceItems.push_back(resourceItem);

	return &resourceItems.back();
}
//resourceItem goes out of scope here. A call to the Release method in the destructor would free resources that should not be freed yet

5. When it comes to variables should I always be giving them a value in the constructor? I have noticed for pointer variables that they do not have NULL or nullptr as there value by default. So it has me wondering if all values are or could be garbage values at there start? Example:


class MyClass {

	public:
  		//Would expect this to be 0 by default. Not sure if I should set this to 0 in constructor because of potential default garbage value
		int count; 
		
  		//Would expect this to be NULL or nullptr, but have seen random garbage value as the default
  		OtherObject *otherObj;

}

 

Advertisement
34 minutes ago, noodleBowl said:

I just have some general questions when it comes to C++ mainly focused on classes

1. When it comes to instantiating a class is there something like a static initializer? Or would it be better to have a static variable(s) and a corresponding flag to say whether or not the variables are ready to use?

Static members are initialized once, automatically.


struct myclass {
	myclass();
	static int mystatic;
};

int myclass::mystatic = 42;

If you need to run arbitrary code in the constructor just once there are facilities for that as well; click.


#include <mutex>

myclass::myclass() {
	static std::once_flag onceflag;

	std::call_one(onceflag, []() {
		// code here will be executed just once
	});
}

 

34 minutes ago, noodleBowl said:

2. Is it "bad" to throw exceptions in the constructor or should I use a 2 step initialization process? Example: I have TruckModel class and when I create a new instance I load in the mesh that this truck model uses. I throw an exception if the file cannot be found because I need the truck mesh in order to continue. This all happens within the TruckModel constructor

Personally I would just do minimal amount of work in the constructor to put the object in a well-defined state. Non-trivial work that can fail goes into separate init functions. But there's nothing technically wrong with throwing from a constructor; the object just isn't constructed and doesn't come into existence in the first place.

34 minutes ago, noodleBowl said:

3. I am currently throwing some exceptions when critical errors happen and I guess exceptions do not work in the way I would expect. I sort of expect the program to come to screeching hault and fully stop execution, but it continues on if you just say to "continue" in the debugger. Simplified example:

Maybe the debugger just allows you to continue execution and "bypass" the exception. Run it without the debugger and see what happens instead.

34 minutes ago, noodleBowl said:

4. Is it possible to hide a destructor from being directly called? I have classes that have a Release method to clean up various pointer resources used by the class instance. The destructor of the class does not call my Release method as that would free the resources prematurely in some cases. Example:

That's the wrong solution to the problem. The core of the problem is that your class is not copyable, but you put a copy of it into the vector. Look up resource ownership, the rule of three/five/zero, and how to properly handle dynamic resources.

My guess is that the class contains a pointer to some resource (or equivalent; you mention a Release method so perhaps a COM-pointer), but you don't implement a proper copy constructor for your class. When you then make a copy, you have two objects holding a pointer to the same resource, and the first one to destruct will also release the resources.

Either implement a proper deep copy (rather than a shallow copy or pointer assignment) so two objects hold separate copies of the actual resource rather than copies of the pointer, or implement proper resource sharing (for example std::shared_ptr), depending on what you want to happen when you copy your objects.

34 minutes ago, noodleBowl said:

5. When it comes to variables should I always be giving them a value in the constructor? I have noticed for pointer variables that they do not have NULL or nullptr as there value by default. So it has me wondering if all values are or could be garbage values at there start? Example:

 

Primitive types are not initialized, you have to do that yourself.


class MyClass {
	public:
		int count = 0;
		OtherObject *otherObj = nullptr;
}

 

Static variables are ok to be initialized in your class implementation file and there are technics to run data once either using STL as Brother Bob mentioned or some static local variable instances for something like read-only constants


typedef union Vector3
{
   public:
      static const Vector3 Zero()
      {
         static Vector3 zero = {{ 0, 0, 0 }}; //<- will be called once
         return zero;
      }
...
  
}vec3;

 

To my personal opinion, exceptions are overworked these days. Sure they are a tool to indicate very hard failure but most of the time a simple bool check/proper return value does the work. If your class is data related, give it operator bool overload and check if it got correctly instantiated. In case of your file example, this would lead to an inline file handle check to zero and then you have your proper state. I try to prevent exceptions whenever possible in my code.

You should design your class in a way that the destructor works as expected, calling it explicitly or not dosent matter here. One might leave local scope without calling your Release function and this is intended by the standard to call at least a classes destructor or default destructor. It is mostly wrong to expect calling a custom function for free up class data when leaving scope (what did not mean that there dosent might exist cases to do so). What you could do here is catching already freed data by doing a simple null-check for your data and call your custom function when the data still exists.

Storing data in a vector could then be done by either of these solutions to create an empty default instance to be passed into your vector and call custom initializer on its reference/pointer or provide a swap mechanism like STL containers do to exchange ownership of whatever is stored inside if you do not want your class to be copyable.

You should provide a basic initialization for your class members if possible at least to prevent nasty side effects (like working on an uninitialized but not null pointer). I use a constructor default if necessary


class MyClass
{
   public:
      inline MyClass() : myHandle(0)
      { }

   private:
      void* myHandle;
};

 

1. Class static members are secretly just global variables of static storage duration.  They are explicitly initialized at their point of definition.

2.The use of exceptions (or not) in constructors is a deeply-held religious belief.  Ask this question and you will be proselytized here.  The right answer actually depends on what paradigm you're programming in.

3. Execution in a debugger is not normal execution. 

4. It is not possible to not fire the destructor on object destruction.  Your design is broken.  Look up the "rule of three" (or "rule of five') and reference counting, or else use the heap if the lifetime of your resource exceeds local scope.

5. All member variables are initialized in the constructor.  If you do not explicitly initialize them in your code, they will have their default constructor invoked.  The default constructor for built-in types like ints and pointers is normally "random" although some compilers in "debug" mode will set a known value.

Stephen M. Webb
Professional Free Software Developer

6 hours ago, noodleBowl said:

4. Is it possible to hide a destructor from being directly called? I have classes that have a Release method to clean up various pointer resources used by the class instance. The destructor of the class does not call my Release method as that would free the resources prematurely in some cases. Example:



ResourceItem *ResourceCreator::createResourceItem(UINT size)
{
	ResourceItem resourceItem;
	system->CreateResourceItem(size, &resourceItem.dataPointer);
	resourceItems.push_back(resourceItem);

	return &resourceItems.back();
}
//resourceItem goes out of scope here. A call to the Release method in the destructor

You could alleviate the problem by using move-semantics:


ResourceItem *ResourceCreator::createResourceItem(UINT size)
{
	ResourceItem resourceItem;
	system->CreateResourceItem(size, &resourceItem.dataPointer);
	resourceItems.push_back(std::move(resourceItem)); // instead of copying resourceItem, move its content into the container, leaving "resourceItem" in an uninitialized state

	return &resourceItems.back();
}
//resourceItem goes out of scope here. A call to the Release method in the destructor

The move-ctor would look something like this:


ResourceItem::ResourceItem(ResourceItem&& item) :
	dataPointer(item.dataPointer)
{
	item.dataPointer = nullptr;
}

Then when the dtor is called for resourceItem, it behaves exactly as if you deleted a default ResourceItem-instance, so you could pack Release in the dtor (if this is what you wanted).

On another note, even without move-semantics, why does this cause a problem in the first place? If you do reference-counting, copying the ResourceItem via copy-ctor should already by incrementing the ref-count by 1 (so on the push_back-line, you have 2 references, when ResourceItem goes out of scope it goes back to 1). In that case, move-semantics would be a tad faster OFC, but it should work eigther way if done right.

For question 4, so I have several classes that have member variables that are COM-pointers. I also have some factory/manager type classes that create these objects and place them into a vector. That issue I was having is that when the local class (the one that is pushed back) goes out of scope the destructor is called, which it should be. But I also had code in there to clean up and free the COM-pointer. Unfortunately this caused the problem of resources being released when they should not

I think I actually have the rule of 5 implace for one of my classes (VertexShader), but I don't believe I have done it correctly as the COM-pointers (shader and inputLayout) that are released on the local copy cause the copy placed into the vector to have a dangling pointers (address is still there, but the object being pointed to does not really exist) and ultimately causes an access violation eventually. This is my current implementation:


//===== Header file for my VertexShader class
class VertexShader
{

public:
	VertexShader();
	~VertexShader();

	std::string id;
	VertexShader(std::string id);
	VertexShader(const VertexShader &other);
	VertexShader(VertexShader &&other);
	VertexShader &operator=(const VertexShader &other);

	//DirectX 11 COM-pointers that I want to free
	ID3D11VertexShader *shader;
	ID3D11InputLayout *inputLayout;

	void release();
};

//====== Implementation file for my VertexShader class
//Normal constructor
VertexShader::VertexShader()
{
	shader = nullptr;
	inputLayout = nullptr;
	std::cout << "Vertex Shader constructor called!" << std::endl;
}

//Normal constructor with id arg
VertexShader::VertexShader(std::string id)
{
	this->id = id;
	std::cout << "Vertex Shader constructor called. Id: " << this->id.c_str() << std::endl;
}

//Destructor
VertexShader::~VertexShader()
{
	std::cout << "Vertex Shader destructor called..." << std::endl;
	release(); //Having release in the destructor currentl gives me in issue :(
}

//Copy constructor
VertexShader::VertexShader(const VertexShader &other) {
	id = other.id + "-COPY-CON";
	shader = other.shader;
	inputLayout = other.inputLayout;
	std::cout << "Vertex Shader copy constructor called. Id: " << id << " Other Id: " << other.id << std::endl;
}

//Move constructor
VertexShader::VertexShader(VertexShader &&other)
{
	id = other.id + "-MOVE_CON";
	shader = other.shader;
	inputLayout = other.inputLayout;
	std::cout << "Vertex Shader move constructor called. Id: " << id << " Other Id: " << other.id << std::endl;
}

//Assignment operator
VertexShader &VertexShader::operator=(const VertexShader &other)
{
	if (&other != this) {
		id = other.id + "-ASSIGN-OPT";
		shader = other.shader;
		inputLayout = other.inputLayout;
	}

	std::cout << "Vertex Shader assignment operator called . Id: " << id << " Other Id: " << other.id << std::endl;
	return *this;
}

//Release method that frees the COM-pointers
void VertexShader::release()
{
	
	if (inputLayout != nullptr)
	{
		inputLayout->Release();
		inputLayout = nullptr;
	}

	if (shader != nullptr)
	{
		shader->Release();
		shader = nullptr;
	}

}

//Factory / Manager class method used to place a VertexShader into a vector
VertexShader *ShaderModule::createVertexShader(std::string id, LPCWSTR fileName, LPCSTR entryPoint, LPCSTR targetProfile, D3D11_INPUT_ELEMENT_DESC *inputElementsDescription, UINT inputElementDescriptionNumElements)
{
	ID3DBlob *shaderCode = compileShaderFromFile(fileName, entryPoint, targetProfile);

	VertexShader vertexShader(id);
	graphicsDevice->device->CreateVertexShader(shaderCode->GetBufferPointer(), shaderCode->GetBufferSize(), NULL, &vertexShader.shader);
	graphicsDevice->device->CreateInputLayout(inputElementsDescription, inputElementDescriptionNumElements, shaderCode->GetBufferPointer(), shaderCode->GetBufferSize(), &vertexShader.inputLayout);
	shaderCode->Release();

	vertexShaders.push_back(vertexShader);
	return &vertexShaders.back();
}
//vertexShader's destructor is called here, which in turn calls the release method. Freeing the COM-pointer object that later cause an access violation


//Example usage of my factory/manager
D3D11_INPUT_ELEMENT_DESC inputElementDescription[] = {
  { "POSITION", 0, DXGI_FORMAT_R32G32B32_FLOAT, 0, 0, D3D11_INPUT_PER_VERTEX_DATA, 0 },
  { "COLOR", 0, DXGI_FORMAT_R32G32B32A32_FLOAT, 0, D3D11_APPEND_ALIGNED_ELEMENT, D3D11_INPUT_PER_VERTEX_DATA, 0 },
};
vShader = GraphicsModule::shaders.createVertexShader("VertexShader", L"default.shader", "VShader", "vs_4_0", inputElementDescription, 2);

 

2 hours ago, noodleBowl said:

I think I actually have the rule of 5 implace for one of my classes (VertexShader), but I don't believe I have done it correctly as the COM-pointers (shader and inputLayout) that are released on the local copy cause the copy placed into the vector to have a dangling pointers (address is still there, but the object being pointed to does not really exist) and ultimately causes an access violation eventually. This is my current implementation:

That's not correct copying. You will end up with two (or more) instances holding the same COM pointer, but the COM object's reference count is not increased to reflect the number of instances holding the COM object. Thus, when the first instance is destroyed, the COM object is released because, as far as it is concerned, the reference count is now zero and there are no more owners.

You need to manage the reference count when you make copies. I believe you do that with


shader->AddRef()

in your copy constructors. This will bump the reference count up when you make a copy of the pointer.

Or, you can use what C++ already offers.


#include <memory>
  
class VertexShader {
  public:
  	VertexShader(std::string id, ID3D11VertexShader *shader, ID3D11InputLayout *inputLayout);
  
  private:
	std::string id;
  	std::shared_ptr<ID3D11VertexShader> shader;
  	std::shared_ptr<ID3D11InputLayout> inputLayout;

}
  
VertexShader::VertexShader(std::string id, ID3D11VertexShader *shader, ID3D11InputLayout *inputLayout) :
  id(std::move(id))
{
  auto com_release_function = [](IUnknown *ptr) {
  	ptr->Release();
  }

  this->shader = std::shared_ptr<ID3D11VertexShader>(shader, com_release_function);
  this->inputLayout = std::shared_ptr<ID3D11InputLayout>(inputLayout, com_release_function);
}

VertexShader *ShaderModule::createVertexShader(...)
{
  VertexShader vertexShader(id);
  
  ID3D11VertexShader *shader;
  ID3D11InputLayout *inputLayout;
  
  graphicsDevice->device->CreateVertexShader(..., &shader);
  graphicsDevice->device->CreateInputLayout(..., &inputLayout);
  
  VertexShader vertexshader(id, shader, inputLayout);
  
  vertexShaders.push_back(vertexShader);
  return &vertexShaders.back();
}

Observe now that the class holds shared pointers that already implements proper sharing and destruction of a shared resource. There is no need to implement a destructor, copying, assignment, release function, or anything that will affect the lifetime of the COM pointers. The std::string manage itself, the two std::shared_ptrs also manage themselves, all automatically.

Regarding exceptions from the constructor, in a good design the constructor should put the object in a reasonably complete and valid state; doing it without throwing exceptions is nicer, but if the constructor needs to do something that throws exceptions, handling exceptions (typically by throwing the towel at a fairly high level) is less inconvenient and less dangerous than handling invalid objects (everywhere and every time).

"Initialization" methods should be present for a specific reason: they are optional, they need to be called multiple times (e.g. to recycle old object in a pool), you don't know whether you want to call them (e.g. mutually exclusive alternatives like digesting two different types of 3D model), they cannot be consolidated into the constructor (e.g. the object is constructed in one thread and completely initialized in another thread), etc.

Omae Wa Mou Shindeiru

6 hours ago, LorenzoGatti said:

Regarding exceptions from the constructor

I feel like in some cases this unavoidable such as my example where I want to create a new mesh, but I can't load it's data from disk and well you can't have the mesh if you can't load in it's data. I guess I'm not really looking to catch and handle the exception. I'm really just looking to explode and say well we exploded cause of XYZ

6 hours ago, LorenzoGatti said:

Initialization" methods should be present for a specific reason

So I have a situation like the following:


class GameWindow {

public
   GameWindow(HINSTSNCE hInstance);
   ~GameWindow();

}

In order to create a GameWindow you must pass in a HINSTANCE. As this variable is required to actually setup the application window. But the compiler gets mad about not having the default no arg constructor. Which makes sense cause I believe if you define a constructor then the compiler does not automatically try to add one for you.

Now I thought I could just add one in and make it private (the default no arg constructor) and then this forces the use of the HINSTANCE version. But then you run into "cannot access" issue due to the constructor being private when you do something like:


class Game {

public:
   Game(HINSTANCE hInstance);
   ~Game();

private:
   GameWindow gameWindow; //compiler does not like this cause the default constructor is private

}

Which also makes sense I guess since GameWindow gameWindow is really just GameWindow gameWindow() right? It does not work like it would in Java or C# where GameWindow gameWindow is just a declaration and the value is null. In C++ it is literally an instance creation of the class

So in this case I would absolutely require an init method right? I can't Because I can't just change the line to use the GameWindow(HINSTANCE hInstance) constructor because that hInstance value has to come from somewhere and be set (not null or a junk value), which I don't think would happen in time in this case

On 10/12/2017 at 1:40 PM, noodleBowl said:

I feel like in some cases this unavoidable such as my example where I want to create a new mesh, but I can't load it's data from disk and well you can't have the mesh if you can't load in it's data. I guess I'm not really looking to catch and handle the exception. I'm really just looking to explode and say well we exploded cause of XYZ

Personally, I would tackle your problem a bit differently.

I would either use a factory class or at the very least a static factory method that does the load / exception logic outside of the model's constructor.  Only once the file could be located and loaded does the factory class or method allocate a model and populate it.

On 10/12/2017 at 1:40 PM, noodleBowl said:

So in this case I would absolutely require an init method right?

No, the problem is how you're declaring the GameWindow variable.  I would have expected something more along the lines of the following instead:


class Game {
public:
  Game(HINSTANCE hInstance) : gameWindow( new GameWindow( hInstance ) ) {}
  ~Game() { delete gameWindow; }

private:
  GameWindow *gameWindow;
}

Or better yet, use a smart_ptr.

This topic is closed to new replies.

Advertisement