some functions in Actor should be const ?

[GoogleCodeExporter]

Hi,

I just started to try Angel 2D. It's great :)

I was trying the message system to detect collisions with my actors. I have two 
type of actors: the main character (TurdusActor) and platforms (PlatformActor). 
When there is a collision between the character and a platform, the world sends 
the message "CollisionWithTurdus", to which Turdus is subscribed.

Then, Turdus can identify the platform by looking at the message sender.

So, the problem is that a lot of getters in Actor are not const. For example:

const String GetName();

After identifying the collided platform, I have a const PlatformActor*, and I 
use it in a function like this:

void TurdusActor::CollideWithPlatform( const PlatformActor* platform, 
b2ContactPoint* contact ) {
    sysLog.Log( "colliding with platform: " + platform->GetName() );
} 

This code doesn't compile in Kubuntu 10.04, using g++ 4.4.3. I think it would 
be more correct to make those functions (GetName(), GetTags(), etc) const 
because they don't change the object. 

The code compiles if I take away the constness of the platform pointer using 
const_cast, but that's not the idea ;)

Also, do you have an "angelic-way" to deal with situations like this?

Thanks!

Eduardo

Original issue reported on code.google.com by eduardo....@gmail.com on 4 Sep 2010 at 5:17

[GoogleCodeExporter]

I forgot to include the compilation error. Here it is:

TurdusActor.cpp: In member function ‘void 
TurdusActor::CollideWithPlatform(const PlatformActor*, b2ContactPoint*)’:
TurdusActor.cpp:99: error: passing ‘const PlatformActor’ as ‘this’ 
argument of ‘const String Actor::GetName()’ discards qualifiers

Original comment by eduardo....@gmail.com on 4 Sep 2010 at 5:18

[GoogleCodeExporter]

I also had a similar issue. You can fix your code by removing const from your 
functions (pretending they will modify the object). But in my opinion it's a 
poor programming practice. I love Angel so far, but I must say that it does not 
make a good use of the C++ const keyword. The API in fact could utilize the 
C++'s type system a little bit more.

Original comment by rhy...@rhywek.com on 6 Feb 2011 at 8:56

[GoogleCodeExporter]

Const is a good programming practice when used correctly, but as pointed out, 
there are some gaps in the current correctness. They're getting fixed bit by 
bit. 

Original comment by lieseg...@gmail.com on 15 Feb 2011 at 2:31

• Changed state: Accepted

[GoogleCodeExporter]

I browsed the source and it looks like Angel devs actually knew which functions 
should be const, but it looks like they just made a small C++ mistake of 
putting the const keyword in the wrong place. Example from Actor.h:

const Vector2 GetSize()

This only makes the returned object constant (not so important, but OK). It 
should be:

Vector2 GetSize()const

In this case the function is constant, meaning the compiler will guarantee that 
it does not modify the actor object. Isn't this what the devs intended?

Original comment by rhy...@rhywek.com on 19 Feb 2011 at 9:37

[GoogleCodeExporter]

That's what const correctness means. It's being addressed, but if you have a 
patch, it would be considered. 

Original comment by lieseg...@gmail.com on 19 Feb 2011 at 1:49

[GoogleCodeExporter]

I can't give a patch as such, as I have modified these files too much. But this 
one liner can do all the magic for fixing Render() functions:

find . -iname '*.cpp' -exec sed -i 's/[:][:]Render[(][)]/::Render()const/g' 
'{}' ';'

A similar one should be run for '*.h' files. And I also used very similar one 
for fixing all the Get...() and Is...() functions, except as I remember not all 
Get...() functions could be made const, the compiler complained for some of 
them, so they need to stay mutable. Example is the function that returns the 
this pointer. But I don't really know why this function exists in the first 
place? Can't one simply use & to get the pointer to any object?

Original comment by rhy...@rhywek.com on 27 Feb 2011 at 11:11

[GoogleCodeExporter]

Hi,
I enclosed a simple patch which should fix constness issues for Actor class, I 
hope it'll be helpful.

Original comment by pell...@gmail.com on 17 Mar 2011 at 1:57

Attachments:

• actor_constness.patch

[GoogleCodeExporter]

Did a const correctness audit/pass that's currently in the trunk. 

Original comment by lieseg...@gmail.com on 19 Mar 2011 at 4:32

• Changed state: Fixed