You said itmar wrote:If you suggest that creating temporaries all over the place plus passing an extra parameter just to work around this is plain stupid.

Moderator: Ras
You said itmar wrote:If you suggest that creating temporaries all over the place plus passing an extra parameter just to work around this is plain stupid.
I just composed a sentence that doesn't make much sense, my english sucks.lucasart wrote:You said itmar wrote:If you suggest that creating temporaries all over the place plus passing an extra parameter just to work around this is plain stupid.
That would be a better design indeed.mar wrote: Or you could split addNode into a private method that gets called recursively and one private method.
Yes that would be more complicated and would result in spaghetti codelucasart wrote: That would be a better design indeed.
My point is always the same: if you have a function that has a side effect on something which you assign the result of the function to, then your design is broken. So fix your broken design instead of looking for clever ways to get around the compiler optimization that breaks your code. Even if you find a workaround the compiler optimization, your design is still broken and your code is still counter intuitive and incomprehensible (not just for the compiler for for humans too).
I proposed my solutionmar wrote:Yes that would be more complicated and would result in spaghetti codelucasart wrote: That would be a better design indeed.
My point is always the same: if you have a function that has a side effect on something which you assign the result of the function to, then your design is broken. So fix your broken design instead of looking for clever ways to get around the compiler optimization that breaks your code. Even if you find a workaround the compiler optimization, your design is still broken and your code is still counter intuitive and incomprehensible (not just for the compiler for for humans too).
The rest is your opinion so please please don't speak for other humans unless you meant yourself and Marco.
Code: Select all
addNode( n->front, nodes, res );
Your design is broken. That's not a point of view. It's a fact. A function takes arguments and returns a value.mar wrote:Yes that would be more complicated and would result in spaghetti codelucasart wrote: That would be a better design indeed.
My point is always the same: if you have a function that has a side effect on something which you assign the result of the function to, then your design is broken. So fix your broken design instead of looking for clever ways to get around the compiler optimization that breaks your code. Even if you find a workaround the compiler optimization, your design is still broken and your code is still counter intuitive and incomprehensible (not just for the compiler for for humans too).
The rest is your opinion so please please don't speak for other humans unless you meant yourself and Marco.
Code: Select all
int tmp = addNode( n->front );
nodes[res].index = tmp;
I never claimed that the code wasn't broken, it was simply because the assignment operator is not a sequence point.lucasart wrote:Your design is broken. That's not a point of view. It's a fact. A function takes arguments and returns a value.
The fact that compiler optimizations break your code should already tell you that your code is broken. Instead of listening you find a "clever" trick to allow your broken code to work anyway. You are starting to sound like Bob Hyatt, when he's 200% wrong but still tries to have the last word. Is it so hard to admit that your design is broken?
A function calling another is not spaghetti code. It's called modularity and is a good thing. Your code is a bunch of spaghettis.
As for the last part about not speaking for humans but only myself and Marco:
- you're a professional programmer, right?
- that means you write code that other people have to modify and vice versa?
- imagine someone else looking at this code:Most likely, they'll just remove the tmp, which seems useless, and break the code in very subtle ways. The result is that the optimized code will be broken but not the debug code. That can be painful to debug.Code: Select all
int tmp = addNode( n->front ); nodes[res].index = tmp;
And regarding more code, again you're wrong. In reality, you would have to spend several lines of comment explaining your ugly hack intended to prevent compiler optimization. If your code was correctly written and designed, it could be understood without comments.
This is my attempt:mcostalba wrote: to be continued...
Code: Select all
struct BSPNode
{
BSPNode *front, *back;
};
struct MapNode
{
int front, back;
};
class Map
{
std::vector< MapNode > nodes;
public:
int addNode(const BSPNode *node, int &idx);
};
int Map::addNode(const BSPNode *node, int &idx )
{
if ( !node )
return -1;
idx++;
nodes.push_back(MapNode{ addNode( node->front, idx ), addNode( node->back, idx ) });
return idx;
}
void test() {
Map map;
BSPNode* root = 0;
int index = - 1;
map.addNode( root, index );
}
Yes this is a nice solution. I prefer root to be first but yes, this code doesn't contain the hack.mcostalba wrote:This is my attempt:mcostalba wrote: to be continued...
Code: Select all
struct BSPNode { BSPNode *front, *back; }; struct MapNode { int front, back; }; class Map { std::vector< MapNode > nodes; public: int addNode(const BSPNode *node, int &idx); }; int Map::addNode(const BSPNode *node, int &idx ) { if ( !node ) return -1; idx++; nodes.push_back(MapNode{ addNode( node->front, idx ), addNode( node->back, idx ) }); return idx; } void test() { Map map; BSPNode* root = 0; int index = - 1; map.addNode( root, index ); }
Note that now root is stored at the end of the vector, don't know if for you is ok.
P.S: Not tested !
Code: Select all
int Map::addNode( const BSPNode *node )
{
if ( !node )
return -1;
int res = (int)nodes.size();
MapNode mn;
nodes.push_back(mn);
mn.front = addNode( node->front );
mn.back = addNode( node->back );
nodes[res] = mn;
return res;
}
Nobody needs to admit anything only because Lucas thinks he can lecture people about "broken designs".lucasart wrote:Your design is broken. That's not a point of view. It's a fact. A function takes arguments and returns a value.mar wrote:Yes that would be more complicated and would result in spaghetti codelucasart wrote: That would be a better design indeed.
My point is always the same: if you have a function that has a side effect on something which you assign the result of the function to, then your design is broken. So fix your broken design instead of looking for clever ways to get around the compiler optimization that breaks your code. Even if you find a workaround the compiler optimization, your design is still broken and your code is still counter intuitive and incomprehensible (not just for the compiler for for humans too).
The rest is your opinion so please please don't speak for other humans unless you meant yourself and Marco.
The fact that compiler optimizations break your code should already tell you that your code is broken. Instead of listening you find a "clever" trick to allow your broken code to work anyway. You are starting to sound like Bob Hyatt, when he's 200% wrong but still tries to have the last word. Is it so hard to admit that your design is broken?