- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Nice one :)
Admin
tl;dr
guessing it's all about the pos?
captcha: vern
Admin
To understand this code, please sacrifice 100 chinchillas to the Priests of the Dark Order.
Admin
honestly the part that really baffles me is this:
protected: T* end_p; void setEndP() { end_p = /* magic */ }
Would this even compile?
end_p = What? Doesn't the = operator require a right value? Maybe I need a few more cards in my magic deck.
Admin
You really need to include an explanation... This time of the day (4pm here) my brain has turned to mush and I can't read a 2 line Basic program, let alone a 80 line C++ abomination
Admin
I'm guessing that's a little censoring done by the poster as he didn't want to expose TOO much of this abomination to us.
Admin
Once you see the confusion between pre- and post-increment, it's not worth even looking at the rest of the code.
Admin
That's one of the neat things about template classes - any functions in the class that you don't use don't actually get compiled. I got bitten by this particular fact in a programming course several years back.
Admin
Groan... I don't even like going through this much of my own code, let alone somebody else's. I need an Executive Summary here, people!
Admin
Yeah, that burned me once, too. The sample input/output given by the professor didn't address a couple functions across a set of templated data structures we had to write, but the grading data most certainly did. At least it drove me to discover unit testing. Too bad it was too late for my 6k LOC assignment that segfaulted on half the grading tests.
Admin
Admin
what are we talking about here.. I don't have a clue!!!
Admin
Admin
While I don't claim to be an uberCoder, can I at least go on record saying that variable names that start with "the" ought to be banned? Oh, I guess I just did. Thank you.
Admin
Compilers are actually obliged to do more than that. Historically, compilers used to do macro-like substitution, but ISO C++98 mandates that the compiler actually check the syntax and semantics of the template body at definition time, rather than at instantiation time, as far as is possible (in effect, everything which isn't dependent on a template type is checked). This sometimes requires adding the template and typename keywords, and "this->" to template bodies.
For those who don't grok C++ or templates, the problem with the code here is that the begin and end iterators are of different types. This means that the iterators can't be passed to most STL algorithms (which require begin and end to be the same type). It's particularly bad because there's absolutely no reason for this madness; end() can and should simply return an iterator / const_iterator.
Having said that, writing good STL-compatible templated containers is hard, and is not for the weak-hearted. The WTF here seems to be that this class clearly wasn't tested in its intended use case -- and this is code which really needs careful testing, because it's hard to get exactly right.
Admin
What confusion? Traditional increment operators are distinguished only by return value. Here, both increments return void, so they have exactly the same effect, as they should.
Admin
Agreed.
Admin
In C# land, you see countless examples of 'myString' or 'MyClass' which are just as descriptive. I guess we have Windows 95 to thank for that.
Admin
Admin
Made me laugh! Thanks
Admin
So there's CIterSTL that is a const iterator. And there's IterSTL, which is a non-const iterator. All good so far. But some genius of course thought that we can inheritate IterSTL from CIterSTL by using the magicness of const_cast<>.
Admin
useless base class
Admin
Look, there's no way to make this sort of code beautiful. But it can be astoundingly useful. They've taken a custom Array class, and made it usable in most algorithms calling for a standard container.
There are a few things that I would have done differently. The naming is horrible, obviously. The object hierarchy is strange. There are a few typedefs you want around when making your own custom iterators that are not present here. Frankly, to be conformant, you want to use something like Boost's iterator_facade.
But this sort of code actually accomplishes something pretty cool. Any generic algorithm written for forward iterators can now be used with this "Array" code refugee from the 1990's. That's pretty impressive to me.
Admin
Well, it looks like a lot of C++ code I've seen and even written. If there are errors, they aren't obvious. There may be some flaws in it (haven't used C++ in anger for too long to tell anymore).
Generally, it's doing something useful. Mostly, it looks like well structured C++ template code. If you don't know C++, you have no hope of even finding anything remotely wrong with it.
It's certainly not a "WTF" - either in its original humorous sense, or in its new, tediously granny-sanitized, sense. In fact, there has been a distinct lack of those brillant "Paula" moments of late...
Admin
While that's technically true, it's not very impressive. It only works as a forward or input iterator. If they had done less work, they could have made it work for reverse and random access iterators too, but they broke those.
It doesn't even work for forward iterator algorithms like std::copy. begin() and end() must be the same type, or convertible to the same type--clearly the first requirement is not satisfied, and I don't know if there's a redacted constructor somewhere that satisfies the second (but as written it shouldn't work).
They could have achieved the same effect (and not broken nearly as much stuff as they have) with:
The only thing they seem to have gained is the ability to assign an object of Array<T> type to an object of Array<T>::IterStl or Array<T>::CIterStl, something they could have achieved by creating a conversion operator to T*--I mean Array<T>::iterator.
Or perhaps Array<T> is really just a C array, so they could be using T* instead.
craaazy
Admin
The thing here is that CIterSTL and IterSTL doesn't do anything a regular T* wouldn't do better. The two begin's should simply return "T*" and "const T*" respectively, and the code would be almost readable.
Admin
But they shouldn't? How are you going to use pre & post increments in a right-hand expression if they just return void? They are supposed to return references to the object being incremented so that you can do this:
foo = ++bar;
Admin
Admin
This depends on the compiler to some degree. Certainly syntax rules need to be followed at all times.
Usually expressions can't be compiled or checked at all until instantiation time, since there's no way to know if type T has member foo() until you actually pick a T. At that time the question becomes "is the method I'm calling instantiated, or the entire class?" and the answer to that question sometimes depends on other questions like "is there a virtual method somewhere in the template class's ancestry?"
Some compilers will (in violation of the standard, apparently) simply treat any identifier token as probably valid, and figure out what they mean at instantiation time. This leads to some interesting (and sometimes useful) results if you have something like:
The above is not allowed by the standard, but was allowed by old versions of GCC. Now you must write:
Really interesting things can happen to your program if Bee declares members that have the same names as things in the global scope, and you change compiler versions.
Admin
And if your class is called Position? I'd much rather see thePosition used than position (note case)
Personally I'd try to describe what the position is of... so somethingPos, but that's just me.
Admin
C is dangerous and C++ is evil. This code snippet actually combines the worst of both worlds.
Admin
Admin
whoever posted this didn't know wtf they were doing either
Admin
NO, the WTF here is that both inner classes (the iterators) are completely useless. They are just wrappers for pointer math. The accomplish nothing except declaring a parent type of a native type that already exists.
The entire thing would work exactly the same if both iterators were removed and the typedefs were changed to:
Admin
So yet another wtf where some developer didn't know that something has already been done, tried to invent it, and managed to fuck it up!
New Rule: no coding unless you can NOT find it on Google!
Admin
Is that the current position, the target position or the original position? Depending on context, simply using position might be bad. Also depending on context using activePosition, thePosition or even myPosition might be a good thing. The semantics of naming variables should follow one rule, do they adequately describe what they are representing without being overly verbose? If so then there is nothing wrong with a variable name. Think about code that needs to detect collision, in this case myPosition is warranted when compared to targetPosition. It is all about context, anyone that wants a hard global rule on these types of naming processes will invariably be wrong.
Admin
Thats a dumb thing to say. Technically its correct (actually its not becuase some of the code is wrong, like the ++ ops) but that misses the point of c++ wrapping. Many STL iteratos are plain pointers but by wrapping them I get:-
a) consistent usage model regardless of underlying code
b) type safety
c) abstraction. I can change this class to use linked lists and code still works. I can add extra checks and debugging code to the iterator which i cannot if i use raw pointers
The sample code has some odd things in it but is pretty good code - nothing that a reasonalbe review could not clean up. op++ is wrong making the non const iter derive from const iter is dumb end detection is broken use vector anyway
Admin
Iä, iä, Codethulhu ftaghn.
Admin
Add me to the "I don't get what's wrong" list.
The writer obviously doesn't know the difference between "++X" and "X++" but apart from that I don't see any glaring "WTF".
To me it's a good example of what Bjarne means when he says "libraries should be easy to use, not easy to write" - it takes something non-standard and adds a wrapper to make it behave in a standard way. It may not be pretty in itself but it improves the code which uses this container - and that's the important code, not the container itself.
Admin
Surely the real blame lies with Windows ME?
Admin
Here is the real C-array as a STL container:
template <class T, size_t N> struct c_array {
typedef T value_type;
typedef value_type* pointer; typedef const value_type* const_pointer; typedef value_type& reference; typedef const value_type& const_reference;
typedef ptrdiff_t difference_type; typedef size_t size_type;
typedef pointer iterator; typedef const_pointer const_iterator;
iterator begin() { return data; } iterator end() { return data+N; }
const_iterator begin() const { return data; } const_iterator end() const {return data+N; }
reference operator[](size_type n) { return data[n]; } const_reference operator[](size_type n) const { return data[n]; }
size_type size() const { return N; }
T data[N]; }
Admin
Admin
And what would your choice be, oh learned apprentice?
(And I can see why you're still an apprentice.)
Admin
actually he/she does understand the differnce - hence the 2 overloads of op++ (most c++ people done get what that means)
if his op++ / ++op returned a value then he would have to make them different, but since they are both void the code is correct in making one call the other
Of course they should be returning T not void
Admin
It looks like they tried to run the example code found that it failed and fixed the != comparator without fixing ==. That example is probably the only usage of the code that was ever tested.
After all, the only time test cases get written is when something break in production so you have to prevent that single breakage and not any other potential breaks. At least that's how my current team seems to work.
captcha: it's been a long long time since I played digdug.
Admin
Admin
It's not totally dumb. It gives you conversion from non-const iter to const iter, and comparison between them, for free. It also means you only have to implement the guts of operator++ etc. once (although the non-const iter still has to override it so it gets the right return type, sadly). It's a neat trick, but certainly not one I'm putting in any of my code -- it's too obscure.
Admin
Agh, I missed that completely! I've been sat here trying to figure out why no one else was mentioning a non const iterator containing a const pointer...
Admin
Okay then, give us an example of one situation where putting the word "the" in front of a variable name adequately describes what it holds without being overly verbose. I'll grant you that it makes sense with myPosition vs targetPosition (although I think a better choice for myPosition would be currentPosition, or even just position since most people would infer that it's the current position when compared to targetPosition). But even granted that, how does thePosition clarify anything, in any circumstances?
As the person who you're calling an idiot pointed out, the word "the" adds no meaning. It could be "the current" position, "the target" position, or "the original" position.
Case sensitivity isn't an argument here either; I know case differentiation makes a lot of VB coders squeamish, but not only is it part of many sets of established code style guidelines, it's actually baked right into the C++/C# specifications by virtue of the fact that static members of a class are accessible from an instance member of a container class with the same name.
Admin
The Real WTF (tm) here is that this code is boring, the WTF is at best mediocre, pedantic, and uninteresting, and as you astutely pointed out, the executive summary is missing leaving us all to dig through the aforementioned boring code to find the uninteresting WTF.