Move time and compiler optimization

[lucasart]

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

You said it

[mar]

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

You said it

I just composed a sentence that doesn't make much sense, my english sucks.
The point is of course that what you propose is stupid.
If you expose a void method, then the caller has to calculate the parameter (which is the size of private vector), which is bad as it requires the caller to do more work than necessary.
Or you could split addNode into a private method that gets called recursively and one private method.
In both cases that results in more code. Less is more and talk is cheap, remember?

[lucasart]

Or you could split addNode into a private method that gets called recursively and one private method.

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).

[mar]

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).

Yes that would be more complicated and would result in spaghetti code
The rest is your opinion so please please don't speak for other humans unless you meant yourself and Marco.

[mcostalba]

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).

Yes that would be more complicated and would result in spaghetti code
The rest is your opinion so please please don't speak for other humans unless you meant yourself and Marco.

I proposed my solution

Code: Select all

addNode( n->front, nodes, res );

Before to read the context you posted, so now I will go to read it

to be continued...

[lucasart]

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).

Yes that would be more complicated and would result in spaghetti code
The rest is your opinion so please please don't speak for other humans unless you meant yourself and Marco.

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:

Code: Select all

int tmp = addNode( n->front );
nodes[res].index = tmp;

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.

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.

[mar]

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:

Code: Select all

int tmp = addNode( n->front );
nodes[res].index = tmp;

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.

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.

I never claimed that the code wasn't broken, it was simply because the assignment operator is not a sequence point.
So instead of talking (which you are obviously good at) I hereby challenge you to show me your solution that you claim would be better.
As you probably realized, passing parent index as argument would not be enough.

[mcostalba]

to be continued...

This is my attempt:

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 !

[mar]

to be continued...

This is my attempt:

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 !

Yes this is a nice solution. I prefer root to be first but yes, this code doesn't contain the hack.
I came up with this in the meantime:

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; 
} 

Which is not as elegant, but in the real code node contains more data that have to be prepared.

[syzygy]

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).

Yes that would be more complicated and would result in spaghetti code
The rest is your opinion so please please don't speak for other humans unless you meant yourself and Marco.

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?

Nobody needs to admit anything only because Lucas thinks he can lecture people about "broken designs".

Martin was so kind to give an example. No need to start a religious war. He is not complaining that the compiler is broken and should have understood what he meant. If you think you can rewrite it in a better way, then just show to do it. No need to be impolite.