for (int i=0; i<(int)MyVector.size(); i++)
SAFE_RELEASE(MyVector);
Should I rewrite my code to use STD algorithm?
Author
I understand from pass post that it is benifical to use the std algorithm. However I already have a number of files that does not use the std algorithm like std::for_each();
My existing code are like this
or it may be slightly more complicated in the for loop, like orthogonalize-ing a list/vector of vertices.
My problem, is for such code, should i rewrite them using std::for_each(), where I have to write another functor().
Especially in cases where it is just a SAFE_RELEASE line/macro in the for loop, writing a functor to excute that seems a little overkill. Or should I just go ahead and convert my code?
What's the goal of refactoring?
Does it work? Would a for_each make it faster?
If you're answer is yes to the first, and no to the second, then you have no good reason to change it. While programming is abstract, and somewhat complex, you're functions work if they work. If they don't, they don't. There's a large pragmatism to it too.
If you're sharing your code with other programmers, and the group would like to use STD library functions and structures as much as possible, then change it. That makes your team more efficient, as they won't be tripping over the reason why your code is shaped that way each time they read it.
"If it ain't broke, don't fix it."
Does it work? Would a for_each make it faster?
If you're answer is yes to the first, and no to the second, then you have no good reason to change it. While programming is abstract, and somewhat complex, you're functions work if they work. If they don't, they don't. There's a large pragmatism to it too.
If you're sharing your code with other programmers, and the group would like to use STD library functions and structures as much as possible, then change it. That makes your team more efficient, as they won't be tripping over the reason why your code is shaped that way each time they read it.
"If it ain't broke, don't fix it."
A couple of comments/questions:
If MyVector does store pointers, then you could avoid manual cleanup entirely by switching to smart pointers. Sticking with the original example though, you could substitute a free 'deleter' function for SAFE_RELEASE and write:
I generally use for_each() when it's fairly straightforward and easy to do so. If however it would require creating some elaborate function object, I just write out the loop manually (but usually using iterators, not indices).
But of course, YMMV.
for (int i=0; i<(int)MyVector.size(); i++) SAFE_RELEASE(MyVector);If MyVector does store pointers, then you could avoid manual cleanup entirely by switching to smart pointers. Sticking with the original example though, you could substitute a free 'deleter' function for SAFE_RELEASE and write:
std::for_each(MyVector.begin(), MyVector.end(), &Delete);I generally use for_each() when it's fairly straightforward and easy to do so. If however it would require creating some elaborate function object, I just write out the loop manually (but usually using iterators, not indices).
But of course, YMMV.
In most situations, I would move to std::for_each. In the situation you outline, however, I would probably use destructors and clear() instead, judging from the semantics you imply.
Last I checked, the entire purpose of using the standard library functions in the first place is to use a good and solid version of algorithms that you don't have to spend the time associated with writing and testing. Thus if the time to create your own things has already been spent, there is really no point in spending even more time to migrate to a set of functions that is designed to save you time. Seems kinda backward.
It looks like I'd rewrite your code to use boost's ptr_vector (!).
To say that macros play poorly with function adapters is an understatement. I'm curious as to what SAFE_RELEASE actually does, in that chances are I'd write it as a function of some sort instead of as a macro.
And, while I probably wouldn't bother to go around rewriting all my loops, I would use BOOST_FOREACH, especially in any situation where you'd end up writing out iterator types, over hand rolled loops where possible.
Or. You know. Stop using C++ when I can. But that's a whole 'nother issue.
To say that macros play poorly with function adapters is an understatement. I'm curious as to what SAFE_RELEASE actually does, in that chances are I'd write it as a function of some sort instead of as a macro.
And, while I probably wouldn't bother to go around rewriting all my loops, I would use BOOST_FOREACH, especially in any situation where you'd end up writing out iterator types, over hand rolled loops where possible.
Or. You know. Stop using C++ when I can. But that's a whole 'nother issue.
Quote:
It looks like I'd rewrite your code to use boost's ptr_vector (!).
I wouldn't.
Yeah, that's right, I just totally disagreed with you! WHAT NOW?
littlekid:
The primary benefit you'd gain from refactoring your for loop into an iterator-based one, or one based on the various "for each" implementations available, would be transparency with respect to the underlying container. For example, you could change "MyVector" to a std::list, and the code would not break (as it stands now, it will, since a std::list and indeed many other SC++L containers cannot be indexed as a vector can).
However (and this is why I disagree with MaulingMonkey's usually spot-on advice), SAFE_RELEASE implies to me the typical "if(ptr != 0) { ptr->Release(); ptr = 0; }" you so often see with COM interfaces such as those one uses when dealing with DirectX. In terms of refactoring bang-for-your-buck, you'll get the most out of using the CComPtr<T> interface to hold those objects you are currently "safe releasing." CComPtr<T> is a COM smart pointer that will handle the calling of the AddRef() and Release() functions for you, using RAII, so you will no longer need to worry about these sorts of "release loops."
I could be wrong (I don't use ptr_vector or the Boost pointer containers much), but my understanding of them is that they (a) clone the objects inserted and (b) provide no "custom deleter" access like the Boost smart pointers do. Which means you would still have to manually be managing the Release() and potentially AddRef() calls (depending on what you can do with the ptr_vector's CloneAllocator) carefully.
Basically, if you refactor your code to use CComPtr<T>, you can actually remove all such loops from your code, which is better than simply rewriting all such loops to use some slightly modified looping paradigm.
Author
How do i exactly use the setup to use the ATL CComPtr?
I added in this header file <atlcomcli.h>, compile and it says GUID_NULL is an unidentified identifier
Must I add the specific atlcomcli etc header for each atl class that i will use? Or is there a generic one that add all of them. I know msdn have a list of atl class listed here:
http://msdn2.microsoft.com/en-us/library/awt7k7f5(VS.71).aspx
Just wondering if there is a single include file that contains all, just being lazy to type :)
I added in this header file <atlcomcli.h>, compile and it says GUID_NULL is an unidentified identifier
Must I add the specific atlcomcli etc header for each atl class that i will use? Or is there a generic one that add all of them. I know msdn have a list of atl class listed here:
http://msdn2.microsoft.com/en-us/library/awt7k7f5(VS.71).aspx
Just wondering if there is a single include file that contains all, just being lazy to type :)
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
