const correctness and others

CaptainPants · Post by **CaptainPants** » Mon Jul 09, 2007 5:42 am

Firstly, if there is a better place for me to post this, please tell me, I wasn't perfectly sure. It is technically a bug I figure.

I've noticed quite a number of situations where members of irrlicht classes are not const when they could be (and should be, in my opinion as a noob to Irrlicht):
e.g.

IGUIElement ---
bool isNotClipped()
IGUIElement* getElementFromPoint(const core::position2d<s32>& point)
virtual bool isVisible()
virtual bool isSubElement()
bool isTabStop()
s32 getTabOrder()
bool isTabGroup()
IGUIElement* getTabGroup() // not sure here
virtual bool isEnabled()
virtual const wchar_t* getText()
virtual core::stringw &getToolTipText() // maybe just have a const version of this one
virtual s32 getID()
bool isMyChild(IGUIElement* child)
bool getNextElement(s32 startOrder, bool reverse, bool group,
IGUIElement*& first, IGUIElement*& closest, bool includeInvisible=false)

virtual void serializeAttributes(io::IAttributes* out, io::SAttributeReadWriteOptions* options=0) // inherited, but still

virtual void draw() // this one is slightly iffy, but logically I think it would be const

I know changing some of these would require a cascade of changes down the inheritance hierarchy. I read some threads from about a year ago regarding const correctness, and I was thinking that if people think it is important maybe there should be somewhere where people can post suggestions about methods to make const? I have noticed lots of 'get' and 'is' methods that are not const.

old threads:
http://irrlicht.sourceforge.net/phpBB2/ ... hp?t=14025
http://irrlicht.sourceforge.net/phpBB2/ ... hp?t=12650 -- more about function arguments, but related

Anyway, I'm just trying to be helpful, I love Irrlicht.

Also a suggestion (so not technically a bug...)
make ReferenceCounter mutable,
make grab() and drop() const
this would allow pointers to const objects to be grabbed

Also:
IGUIElement::getRelativePosition() and getAbsolutePosition()
both return rect<s32> so I wonder if they should be called getRelativeRect() and getAbsoluteRect()

bitplane · Post by **bitplane** » Tue Jul 10, 2007 6:35 am

well, with irrlicht trying to remain newbie friendly, Niko has always expressed a dislike for const correctness as it just makes the api harder for beginners to extend and doesn't offer any real protection.
we're avoiding API changes wherever possible, so I've not made the new methods in IGUIElement const because then we'd have (even more of) an untidy mix of const and non-const members without a good explanation for it.

we should definitely revisit this though, keep the complaints coming in and i'm sure it will happen eventually

CuteAlien · Post by **CuteAlien** » Tue Jul 10, 2007 10:33 am

Unfortunately it makes writing my own applications harder when Irrlicht is not const correct. I can do either very ugly const casts or stop programming const correct code myself when using Irrlicht. One of the very first things I did when starting to use Irrlicht was making a lot of functions const. But I noticed that there was not much interest in that though until now I never really knew why. And after a few updates I did give up, as it made updating a lot more work.

I would prefer const correct code very much. It might not offer real protection, but it's a very valuable tool. Nearly every time I had a const problem so far, it did upon examination reveal some deeper problem in my design. This is for me maybe even more important than the documentational effect. But especially for an API it would also be useful to have const just as help for documentation. It does give additional valuable information about members functions for the cost of typing a single word!

Break the API for that - I'm all for it :-)

CaptainPants · Post by **CaptainPants** » Tue Jul 10, 2007 11:29 am

I can agree with not wanting to break everyones code, but I'd have to say that I think const correctness is a very good thing. And I think that virtually all the methods of IGUIElement that I listed could be made const with no effect on code beyond having to add the word const in any overriding methods.

The main problem I guess is that compilers won't pick up the change in method signature as an error, so any overriding methods will have to be hunted down by hand or they .. won't override.

When you're using pointers a lot const correctness is a bit harder I guess, especially as atm IUnknown's reference counting system doesn't work for const objects its pretty impossible to use the engine in a const correct manner at all. (solution in previous post, sorry to keep pushing this)

Also, the list data structure doesnt have const iterators, but allow their data to be editted via iterators even when const.

I know thats not so much an argument for const correctness as a concept, but it seems a bit .. broken.

Any thoughts would be welcome.

hybrid · Post by **hybrid** » Tue Jul 10, 2007 11:48 am

Some const parameters will come in when the animation system is merged with core Irrlicht. As said elsewhere I assume this to break nearly every usercode (as long as we don't want to keep both APIs in) and will be a very good point to make omse struct parameters (such as SEvent) a const& parameter.
The problem with const methods is that you cannot put updating calculations inside Getters unless you make the attributes mutable. And IMHO, mutable should be used only in very few situations. So you cannot make deferred updates and thus move expensive calculations out of timing critical code that easy. However, Niko also never reverted any const additions except for the SEvent in OnEvent. So we'll just add some more const's next time and it will develop slowly and eventually become correct.

CaptainPants · Post by **CaptainPants** » Wed Jul 18, 2007 4:36 am

Upon reflection, keeping const correctness when you're using pointers and templated data structures is impossible without some really yucky code allowing for example:
list<T *> to be viewable as a const list<const T *>&..
which is actually possible but quite horrible.

I still do believe that the list and map data structures need to const correct, as with any other struct types that arent generally stored in a list of pointers.

But basically anything descending from IUnknown probably need not be correct, because it would not help in most situations. In fact I'd probably advocate the removal of all const qualifiers on methods of any descendant of IUnknown in the engine. (I'm only referring the const methods, not constness of arguments and return values).

The upshot of this:
Please make these structures have const iterators.. map at the moment just doesnt work as a const reference, and because of how it's written may never. As far as list goes, if I submitted a version of the list data structure(using the irrlicht list code) that had const iteration is there a chance anyone would look at it with regards to sticking it in the engine?

So I'm personally using stl structures anywhere that I can in my code. This is messy, because it will probably eventually result in me either having to edit engine code or do expensive copies between types of lists.

Anyways, you probably won't hear any more from me on this topic.

hybrid · Post by **hybrid** » Wed Jul 18, 2007 7:56 am

I don't think that the situation will become better if you don't keep on posting issues and solutions. Patches to Irrlicht code can be always submitted, the best place is the sourceforge patch tracker (because larger updates could be lost in this forum otherwise).

CuteAlien · Post by **CuteAlien** » Wed Jul 18, 2007 4:30 pm

CaptainPants wrote: So I'm personally using stl structures anywhere that I can in my code. This is messy, because it will probably eventually result in me either having to edit engine code or do expensive copies between types of lists.

I also worried about that at first. But it causes rarely problems. I usually had never to exchange lists between my application and Irrlicht and I think this will be the case for most applications.

String conversion between stl and irrlicht did happen more often (because of gui-code), but it was also never hard to handle.

I'd recommend working with stl in your own code anyway until you have a really good reason to do otherwise. So you can use your code even if you switch other tools like the engine. And it will work with most libraries and usually you need more than one library in the long run. Just don't use it for patches to Irrlicht :-)

As for map ... that's really another topic.