Name internal functions differently than documented functions

Started by wizzard, 19 September 2013, 14:17:07

wizzard

I do a lot of template work with TGUI since I expose it to the Squirrel scripting language using them.

When I look at the documentation, I see functions like the following
tgui::Container::focusWidget
tgui::Container::remove

These functions have undocumented (internal) overloads that are exposed.
So, when I go to shove them into my templates they do not work because of template ambiguity.
With these functions, I can just define the template of the function I want bound.
Anyways, the problem I have is that it took awhile for me to realize they had internal overloads.
In addition, I'm pretty sure IDE auto-completion will see the internal overloads and recommend them.

SFML handles internal functions by putting them in a "priv" namespace.
I think it'd be a good idea to instead append "Internal" to all the internal functions.
(e.g. tgui::Container::focusWidgetInternal)

texus

I have to look into this, because it isn't that easy and it definately isn't a small change.
The difference with sfml is that my classes highly depend on each other, so it can't be fixed that easily.

If I would be able to declare a namespace inside a class it would be fixed a lot faster, but I can't so I'll have to look for other solutions.

wizzard

Quote from: wizzard on 19 September 2013, 14:17:07
I think it'd be a good idea to instead append "Internal" to all the internal functions.
That was meant as an alternative to the way SFML does it.
I do not think at this point there is any benefit to doing it the same way as SFML.
No namespace is needed - just renaming the internal versions of the functions and
renaming all the places where the internal versions are called.

texus

Ah, I misread your first post. I read that you didn't find it a good way to add internal to the functions.
But I think I'm just going to do it for the duplicate functions at first (focusWidget, get, remove, ...) and not for functions like initialize, mouseOnWidget, leftMousePressed, ...

Anyway that would just be temporary until I get a better design and can do a better job.


But I see problems with removing some of the internal functions. I even have reasons to no longer make them internal.
In the widget callbacks, I am forced to use a Widget* instead of Widget::Ptr (to avoid the big memory leak that I had). But that means that Container will need a remove function that takes a Widget*. But I also want you to remove a Widget::Ptr without first calling the get function on it to retrieve the Widget*. That is why I believe that both remove functions should stay.

texus

The best option that I can think of so far is name the functions like this:
remove(Widget::Ptr);
removeWidget(Widget*);


But I took a look at the get functions and I don't even want to change that.
Although I do realize that the template one could be dropped and that you could type tgui::Button::Ptr(gui.get("widget")) instead of gui.get<tgui::Button>("widget").

wizzard

Making certain functions no longer internal fixes this problem too

My issue is that the documentation isn't matching results when using templates

wizzard

I'm sorry. I actually have no issue with the get function
That is documented just fine. That was my error and I will remove it from the original post

Edit: To reiterate my problem with the two functions in the original post,
the thing they have in common is that only one of their overloads is documented

texus

Ah, I though that the problem was that there were two functions with the same name.

So making the remove and focusWidget functions no longer internal (which I needed to do anyway) will solve this whole discussion? I was starting to to get afraid that I would have to change a lot of code.

wizzard

I have not ported all of TGUI and so I do not know if those are the only two functions that feature this issue

Those are the only ones I've run into so far though

Edit: ... and yes, making them no longer internal fixes the issue

texus

I don't think there are any other, but if you find others than just let me know.

Changes to these functions and the updated documentation will be uploaded in half an hour or so.