Hard Light Productions Forums
Off-Topic Discussion => Programming => Topic started by: Uchuujinsan on November 18, 2009, 08:02:54 am
-
void Unregister(IEngineModule* module)
{
for (std::vector<IEngineModule*>::iterator i = engineModules_.begin(); i != engineModules_.end(); ++i) {
if (*i == module) {
engineModules_.erase(i);
break;
}
}
}
Can anyone of you imagine a situation where this code could throw?
Would be important because it's a function that gets called in a destruktor.
I don't know if I should just wrap it in a try/catch block, let remain it like this because it won't throw anyway, wrap the call in a try/catch block or look for another solution.
[edit]
Seems like "erase" can't throw, so that would leave "*i" as a possible problem..?
-
Is there any particular reason why this code is using the vector iterator in the first place? Seems like a better idea to just do:
for(int i = 0; i < engineModules_.size(); ++i)
{
if(engineModules_[i] == module)
{
engineModules_.erase(engineModules_.begin()+i)
break;
}
}
In addition, whenever code is using a for loop to loop through an array to find and delete something, there's a better way to do it.
Also, this code doesn't check to see if the module that's sent to the function is a NULL pointer, which could blow something up.
-
I'm always using iterators if I'm iterating through a STL container, because it's typed it will make it harder to do stupid things with it.
And engineModules_.begin()+i is using the vector iterator as well - begin() returns an iterator :/
In addition, whenever code is using a for loop to loop through an array to find and delete something, there's a better way to do it.
Could you further explain this?
Also, this code doesn't check to see if the module that's sent to the function is a NULL pointer, which could blow something up.
The pointer here is never used to access something in that function, so it shouldn't be able to make any problems.
-
(engineModules_.begin()+i) is only using an iterator because there's not other way of deleting something.
The pointer here is never used to access something in that function, so it shouldn't be able to make any problems.
But if it IS a null pointer then suddenly you could have a situation where (engineModules_[ i] == module) evaluates to TRUE when both are NULL and then everything goes to hell.
Could you further explain this?
Well the problem is simply that goign through a for loop to find something is always the least efficient way of doing it. It depends on how its used elsewhere and how often the remove funciton is being called. Usually when you are iterating through an array you need to consider whether or not it would be better as a linked list of some sort.
-
(engineModules_.begin()+i) is only using an iterator because there's not other way of deleting something.
Well, then I can use iterators from the beginning anyway, makes it easier to switch to another container, because a list for example doesn't support engineModules_.begin()+iThe pointer here is never used to access something in that function, so it shouldn't be able to make any problems.
But if it IS a null pointer then suddenly you could have a situation where (engineModules_[ i] == module) evaluates to TRUE when both are NULL and then everything goes to hell.
Well, if both are null at a completely valid position i, than that value is supposed to be removed at that position i, and it will be removed.
Could you further explain this?
Well the problem is simply that goign through a for loop to find something is always the least efficient way of doing it. It depends on how its used elsewhere and how often the remove funciton is being called. Usually when you are iterating through an array you need to consider whether or not it would be better as a linked list of some sort.
[/quote]
Ah, you don't mind the iterating itself, but the possible erase call that is inefficient with a vector (iterating through a list isn't faster).
Well, that's right, I could change it to a list, it's been a vector before that erase call got added.
-
(iterating through a list isn't faster).
Yes it is. Regardless of that, however, removal from a list is a O(1) operation if its doubly linked.
Well, then I can use iterators from the beginning anyway, makes it easier to switch to another container, because a list for example doesn't support
If I were implementing a list I'd just do it manually, its faster that way and its not very hard.
Well, if both are null at a completely valid position i, than that value is supposed to be removed at that position i, and it will be removed.
Well, no, you could hit a fake NULL value and have that removed instead, but yes it would never throw an actual error.
...
Going back to the original problem, does Erase call an object's destructor? Your error might be in there instead.
-
I don't have an error, I want to prevent that while an exception gets thrown a destructor call will be invoked that might throw a second exception while the first one is still in effect, aborting the programm and preventing me from handling the error. And that's why I want to ensure that this code doesn't throw and was asking if I should just use try/catch (maybe if I might change that code later, so I can easily introduce a throw), or leave at that because it can't throw anyway.
I couldn't see a possibility how it could throw, and was curious if I was right because I wasn't that sure.
-
Pointers don't have destructors. *i should work as long as the iterator is valid. Trying to treat a container generically is usually a bad idea.
You probably should be using something like remove_if (http://www.sgi.com/tech/stl/remove_if.html) if you want to do it properly though.
-
... Please read from the beginning.
I resolved the question now, but I post an example anyway:
void Unregister()
{
std::vector<int> tmp;
tmp.push_back(3);
}
void Bar()
{
IEngineModule IEM;
throw "blubb";
}
class IEngineModule {
IEngineModule()
{
}
~IEngineModule()
{
Unregister();
}
};
int main()
{
try {
Bar();
}
catch(...)
{
std::cout<<"Error"; //Under certain circumstances this line won't be reached
}
std::cout<<"Success"; //and neither will be this one(!)
}
Which is a bad thing to happen.
Further read here:
http://www.parashift.com/c++-faq-lite/exceptions.html#faq-17.3
-
I did read from the beginning, but now this thread reminds me of this wtf (http://thedailywtf.com/Articles/My-Tales.aspx).
-
Well, I don't have to pull the plug, a simple "out_of_memory" exception by tmp.push_back(3) will result in terminate() being called :>
-
Given that sensible stl implementations would allocate some space initially you'd probably get your exception in the constructor instead.
On a more general level, what do you really expect to do if you run out of memory?
-
We are already at a hypothetical level, because that code is save, so..
Most reasonable would be to free memory, save data, and end the programm (with an error message). But the main reason is, that I - in this case - don't care about the out of memory itself, if I would I'd have to have a lot of more try/catch in my code, but about the possibility to debug some other part of the code that threw an (unknown) exception I never get to see because of terminate().
If std::vector<int> tmp; already throws, that wouldn't change the problem at all.
I simply have to ensure that an exception can never leave the destructor, and that was what I was trying to check her..
-
How could you decide what memory to free? Would the files to be saved need to be opened requiring more memory?
-
That depends on the application, but normally you can always nuke the graphics routine and unload everything there - if you are going to crash anyway, that shouldn't be a problem. In fact you can design the programm in such a way, that if you get an out of memory exception, the stack unwind will free everything that's not crucial data by iteself. Then catch it and use the freed memory to save that crucial data.
That works as long as an destructor during stack unwind won't throw :>