My question is more like a doubt.
Few days ago I started organizing my code in a way that I wasnt very used to, but I liked it a lot, the problem is I dont know if its bad or if its good, I mean its working but I wonder if the code gets bigger it will eventually be problem. This how I'm doing with my code:
//just the functions part of the header, without includes and stuff
.h file
.cpp file
will I get errors or anything weird in the future using it like this?
also, putting "static" on the mydata pointer will make it better? or its just
better to put this struct and pointer on the .h file?
Few days ago I started organizing my code in a way that I wasnt very used to, but I liked it a lot, the problem is I dont know if its bad or if its good, I mean its working but I wonder if the code gets bigger it will eventually be problem. This how I'm doing with my code:
//just the functions part of the header, without includes and stuff
.h file
1 2 | void init(); void someFunction1(); |
.cpp file
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 | struct myData
{
int someVar = 0;
int someVar1 = 21;
}
myData* mydata = nullptr;
void init()
{
mydata = new myData();
mydata->someVar = 1;
}
void someFunction1()
{
mydata->someVar = mydata->someVar1;
}
|
will I get errors or anything weird in the future using it like this?
also, putting "static" on the mydata pointer will make it better? or its just
better to put this struct and pointer on the .h file?
Edited by khofez
on
It's kind of do this when it is really needed, but it might lead to problems in future - what if you want multiple myData instances?
It's usually much better to explicitly pass data pointer back and require for user to pass it back later:
It's usually much better to explicitly pass data pointer back and require for user to pass it back later:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 | // header file
struct myData;
myData* init();
void someFunction1(myData* data);
// cpp implementation
struct myData
{
...
};
myData* init()
{
// allocate and return myData
}
void someFunction1(myData* data)
{
....
}
|
Also thank you for your clear response, I mean, i'm sorry if I cant reference to another website here i dont know if I can, but I asked this on stack overflow and the guys just throwed at me comments like "dont do that, why are you doing that? why dont you use PROPER CLASSES" or something like "dont use functions that take no parameters and return void" (which I think its non-sense, why someone would not use a void function with no parameters), anyway
This comes down to a design question: is there are reason to force your API design to only use that single pointer? The way that mmozeiko provided the API, you can still implement by only every returning a single pointer to the same instance. However, that API can evolve easier.
As to why do not use a void function and no parameters: the function's only purpose is to create side-effects in some system. These types of APIs have a natural tendency to evolve into hard to maintain systems that are extremely inflexible. Over time, they can easily become a huge system of interrelated parts that are hard to understand and introduce subtle interdependencies that were not intended.
Even doing the first step in mmozeiko's layout, the implication is you are modifying the data that you sending in vs. modifying some data in some nebulous area.
Update
Another way to think about it is your functions are a pipeline for manipulating data. In general, when it's clear what the "data-in" and the "data-out" is, the usage of the API and the changes that happens can be better reasoned.
vs.
Even if we only ever have a single matrix instance in the entire program, the second function is provides an implicit contract based on it's signature: it multiplies the specific Matrix by the given Scalar.
The first API, which matrix is being multiplied? Is this some sort of API for an OpenGL-like state machine? How do I specify which matrix to multiply? Etc...
As to why do not use a void function and no parameters: the function's only purpose is to create side-effects in some system. These types of APIs have a natural tendency to evolve into hard to maintain systems that are extremely inflexible. Over time, they can easily become a huge system of interrelated parts that are hard to understand and introduce subtle interdependencies that were not intended.
Even doing the first step in mmozeiko's layout, the implication is you are modifying the data that you sending in vs. modifying some data in some nebulous area.
Update
Another way to think about it is your functions are a pipeline for manipulating data. In general, when it's clear what the "data-in" and the "data-out" is, the usage of the API and the changes that happens can be better reasoned.
1 | void multiplyMatrix(int Scalar); |
vs.
1 | void multiplyMatrix(int Scalar, matrix* Matrix); |
Even if we only ever have a single matrix instance in the entire program, the second function is provides an implicit contract based on it's signature: it multiplies the specific Matrix by the given Scalar.
The first API, which matrix is being multiplied? Is this some sort of API for an OpenGL-like state machine? How do I specify which matrix to multiply? Etc...
Edited by David Owens II
on
Sure, now you need only one. That is fine. Then use one. But if you look think about it - there's no reason that you need only one renderer. What if you'll need to render to multiple Windows separately? What if you'll need to support OpenGL and Vulkan (or some version of D3D). This is such a simple change, but will make your life much easier when you'll need to do it. That's why keeping less global state is better - its just better to read and understand such code, when you see what data it accepts and is able to operate. Keeping "hidden" state (global variables) makes your code harder to think about, it can break in many ways you don't expect.
Edited by Mārtiņš Možeiko
on
I would encourage you to keep going with your current approach. You currently have a global Renderer object (or myData), and lots of people will tell that it's poor design to use global variables. Maybe one day you will need two Renderer objects, and you will have to do a lot of work to re-design the program to avoid using a global variable. But then you'll deeply understand why people often tell you to avoid global variables.
On the other hand, maybe you'll never need a second renderer and therefore a global variable will continue working well. If so, congratulations, you avoided over-complicating your code by adding the flexibility to support multiple renderers.
As you get more experience, you'll start to get a feeling for when it's good to add flexibility to your design at the start, and when it's better to write the simplest thing that works. A lot of programmers fall into the trap of making every bit of code incredibly flexible for possible future improvements, but this makes their designs more complicated! And most of those future improvements wiIl never be needed anyway. It's better to err on the side of writing the simplest thing possible at any moment.
(If you want my opinion, this is probably one of the cases where the extra flexibility is worth it, because the complexity 'cost' is so low).
If you are going to extend the code to work with multiple Renderer/myData instances, Mārtiņš's code shows you how to do that with opaque pointers so that the struct is only visible in the .cpp file. I used to do that, too, and it's an approach I read about it in the book ‘C Interfaces and Implementations’. These days I just put the struct in the header file instead. There's a small risk that users of the header file will modify the struct (which is not possible if you have opaque pointers), but usually I am the user and I don't do anything silly.
On the other hand, maybe you'll never need a second renderer and therefore a global variable will continue working well. If so, congratulations, you avoided over-complicating your code by adding the flexibility to support multiple renderers.
As you get more experience, you'll start to get a feeling for when it's good to add flexibility to your design at the start, and when it's better to write the simplest thing that works. A lot of programmers fall into the trap of making every bit of code incredibly flexible for possible future improvements, but this makes their designs more complicated! And most of those future improvements wiIl never be needed anyway. It's better to err on the side of writing the simplest thing possible at any moment.
(If you want my opinion, this is probably one of the cases where the extra flexibility is worth it, because the complexity 'cost' is so low).
If you are going to extend the code to work with multiple Renderer/myData instances, Mārtiņš's code shows you how to do that with opaque pointers so that the struct is only visible in the .cpp file. I used to do that, too, and it's an approach I read about it in the book ‘C Interfaces and Implementations’. These days I just put the struct in the header file instead. There's a small risk that users of the header file will modify the struct (which is not possible if you have opaque pointers), but usually I am the user and I don't do anything silly.
Thanks guys for all the responses, now this is more clear to me.
Oh and also I'm the only one using this code, the design of this API its only for me, so I dont see a reason to design it to be "bullet proof" for other users, i'm not willing to share it, and i like to keep things simple, and there is no risk to anyone modifying it
Oh and also I'm the only one using this code, the design of this API its only for me, so I dont see a reason to design it to be "bullet proof" for other users, i'm not willing to share it, and i like to keep things simple, and there is no risk to anyone modifying it