Re: attack of silly coding standard?
Stuart Golodetz <blah@blah.com> writes:
Well you ALWAYS return modification anyway, why not just
if (...)
else if(...)
else if(...)
return modification;
Because I don't have the things I want to test available at the right
points, e.g. I want to test ancestor or descendants, but I need to
calculate them first. I don't want to calculate them at the start of
the function, because that might not be necessary. I could write it
as:
PFNodeID ancestor;
std::stack<PFNodeID> trail;
...
else if((ancestor = find_ancestor_in_representation(node, trail)) !=
PFNodeID::invalid())
{
...
}
...
etc., but then I have to create all the required objects up-front,
possibly unnecessarily. I'm not sure it's really clearer that way.
I don't get your point, if you do
if (...)
else {
std::stack<...>
}
whenever the first condition is false I never compute anyway the values
below.
There is also some not nice duplication like
m_listeners->node_was_deselected(node, commandDepth);
And that line also is very suspicios.
For my understanding something called
"node_was_deselected" should be BOOL, and thus not being discarded like
this.
That thing I guess has some side effects then, not very nice for a
function called like that...
It might well do too many things :) So perhaps breaking it into
multiple functions might be an improvement, I'm not sure. Still wonder
whether that's really clearer though. Not sure what you're thinking of
in terms of polymorphism here -- could you elaborate?
Well I don't know the problem enough, but for example you can divide
ancestors and descendants in two separate sets maybe..