• Raptor (unregistered)

    Nice one :)

  • boo (unregistered)

    tl;dr

    guessing it's all about the pos?

    captcha: vern

  • KattMan (cs)

    To understand this code, please sacrifice 100 chinchillas to the Priests of the Dark Order.

  • KattMan (cs)

    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.

  • DOA (unregistered)

    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

  • Hit (unregistered) in reply to KattMan
    KattMan:
    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.

    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.

  • dave (unregistered)

    Once you see the confusion between pre- and post-increment, it's not worth even looking at the rest of the code.

  • Quietust (unregistered) in reply to KattMan
    KattMan:
    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.

    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.

  • bstorer (cs)

    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!

  • bstorer (cs) in reply to Quietust
    Quietust:
    KattMan:
    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.

    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.

    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.

  • dkf (unregistered) in reply to bstorer
    bstorer:
    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!
    Executive Summary: All your soul are belong to us! Bwahahahaha!
  • topcat_arg (cs)

    what are we talking about here.. I don't have a clue!!!

  • dkf (unregistered)
    Comment held for moderation.
  • Duston (unregistered)

    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.

  • Richard (unregistered) in reply to Quietust
    Quietust:
    KattMan:
    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.

    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.

    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.

  • jim (unregistered) in reply to dave
    dave:
    Once you see the confusion between pre- and post-increment, it's not worth even looking at the rest of the code.

    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.

  • SomeCoder (unregistered) in reply to dave
    dave:
    Once you see the confusion between pre- and post-increment, it's not worth even looking at the rest of the code.

    Agreed.

  • bobbo (unregistered) in reply to Duston
    Duston:
    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.

    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.

  • freelancer (cs) in reply to Duston
    Duston:
    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.
    What's your problem with "the"? I usually just prepend "the" instead of "active" because it's shorter. Example: thePosition instead of activePosition
  • H|B (cs) in reply to bobbo
    bobbo:
    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.

    Made me laugh! Thanks

  • jlh (unregistered)

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

  • phoniq (unregistered)

    useless base class

  • Not a WTF (unregistered)

    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.

  • Matt Palmer (unregistered)

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

  • Zygo (unregistered) in reply to Not a WTF
    Not a WTF:
    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.

    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:

        typedef T* iterator;
        typedef const T* const_iterator;
    

    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

  • JEBreimo (unregistered)

    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.

  • MaGnA (cs) in reply to jim
    jim:
    dave:
    Once you see the confusion between pre- and post-increment, it's not worth even looking at the rest of the code.

    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.

    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;

  • benryves (cs) in reply to freelancer
    freelancer:
    Duston:
    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.
    What's your problem with "the"? I usually just prepend "the" instead of "active" because it's shorter. Example: thePosition instead of activePosition
    What's wrong with just "Position"?
  • Zygo (unregistered) in reply to Quietust
    Quietust:
    KattMan:
    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.

    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.

    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:

    struct Bee {
        int a;
    };
    
    template <class B>
    struct Foo : public B {
        int get_a_from_B() const { return a; }
    };
    
    Foo<Bee> foo;
    foo.get_a_from_B();
    

    The above is not allowed by the standard, but was allowed by old versions of GCC. Now you must write:

    template <class B>
    struct Foo : public B {
    // either:
        using B::a;
        int get_a_from_B() const { return a; }
    // or:
        int get_a_from_B() const { return B::a; }
    // or maybe even:
        int get_a_from_B() const { return this->a; }
    };
    
    Foo<Bee> foo;
    foo.get_a_from_B();
    

    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.

  • Khanmots (unregistered) in reply to benryves

    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.

  • An apprentice (unregistered) in reply to bstorer
    bstorer:
    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!

    C is dangerous and C++ is evil. This code snippet actually combines the worst of both worlds.

  • Not so sure (unregistered) in reply to An apprentice
    An apprentice:
    bstorer:
    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!

    C is dangerous and C++ is evil. This code snippet actually combines the worst of both worlds.

    So is a kitchen knife - in the wrong hands. It's not the tool, it's the hand holding it!

  • phoniq (unregistered)

    whoever posted this didn't know wtf they were doing either

  • Bryan (unregistered)

    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:

        typedef T* iterator;
        typedef T const* const_iterator;
    
  • snoofle (cs) in reply to Bryan
    Bryan:
    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:

        typedef T* iterator;
        typedef T const* const_iterator;
    
    .

    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!

  • KattMan (cs) in reply to benryves
    benryves:
    freelancer:
    Duston:
    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.
    What's your problem with "the"? I usually just prepend "the" instead of "active" because it's shorter. Example: thePosition instead of activePosition
    What's wrong with just "Position"?

    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.

  • pm (unregistered) in reply to Bryan

    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

  • Island Usurper (unregistered)

    Iä, iä, Codethulhu ftaghn.

  • Joce (unregistered)

    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.

  • DaveK (cs) in reply to bobbo
    bobbo:
    Duston:
    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.

    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.

    Surely the real blame lies with Windows ME?

  • Aron (unregistered)

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

  • Faxmachinen (cs) in reply to KattMan
    KattMan:
    benryves:
    freelancer:
    What's your problem with "the"? Example: thePosition instead of activePosition
    What's wrong with just "Position"?
    Is that the current position, the target position or the original position?
    Yeah, adding "the" to "position" really clears everything up.
  • mrprogguy (cs) in reply to An apprentice
    An apprentice:
    bstorer:
    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!

    C is dangerous and C++ is evil. This code snippet actually combines the worst of both worlds.

    And what would your choice be, oh learned apprentice?

    (And I can see why you're still an apprentice.)

  • pm (unregistered) in reply to Joce

    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

  • DK (unregistered)
    bool operator== (const T* const position) const { return CIterSTL::thePos == position; }
      bool operator!= (const T* const position) const
        { return ConstIteratorSTL::thePos != position; }
    
    //  Array<Int> array(s);
    //  for ( Array<Int>::iterator iter = array.begin(); iter != array.end(); iter++ ) {
    //    *iter += 1;
    //  }
    

    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.

  • KattMan (cs) in reply to Faxmachinen
    Faxmachinen:
    KattMan:
    benryves:
    freelancer:
    What's your problem with "the"? Example: thePosition instead of activePosition
    What's wrong with just "Position"?
    Is that the current position, the target position or the original position?
    Yeah, adding "the" to "position" really clears everything up.
    And you sir suffer dearly from the "I wanna make a post without reading the post I am responding to" disease. This, regretfully makes you a total idiot. Hopefully by responding you will overcome this disability and realize that I addressed this problem in the portion of the post you felt necessary to remove.
  • Richard (unregistered) in reply to pm
    pm:
    making the non const iter derive from const iter is dumb

    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.

  • Devi (cs) in reply to pm
    pm:
    making the non const iter derive from const iter is dumb

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

  • Aaron (unregistered) in reply to KattMan
    KattMan:
    Faxmachinen:
    Yeah, adding "the" to "position" really clears everything up.
    And you sir suffer dearly from the "I wanna make a post without reading the post I am responding to" disease. This, regretfully makes you a total idiot. Hopefully by responding you will overcome this disability and realize that I addressed this problem in the portion of the post you felt necessary to remove.
    Your original post didn't address that at all. It was just a general statement saying "in some situations it might be better".

    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.

  • joe_bruin (cs) in reply to bstorer
    bstorer:
    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!

    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.

Leave a comment on “History++”

Log In or post as a guest

Replying to comment #:

« Return to Article