[Development] Some Qt3D feedback

Stephen Kelly steveire at gmail.com
Wed Jun 10 21:03:32 CEST 2015


Sean Harmer wrote:

>> 1) The include/Qt3DCore/Window file doesn't have a Q prefix.
>> 
>> as every other header does. Should probably be Qt3DWindow.
> 
> Right, this actually needs removing and something temporary putting in
> place in the examples for now. 

> I'll try to tidy up and move the existing Window thing to the examples
> later today or tomorrow.

It looks like it's still there. Is it going to be released like that?

Would it be useful to request an API review for this new module at this 
point? I don't know if there are other new modules in Qt 5.5.

>> 2) A private header is included in a public header:
>> 
>>  include/Qt3DCore$ grep private/ *.h
>>  qaspectjobmanager.h:#include <Qt3DCore/private/qt3dcore_global_p.h>
>> 
>> This is concerning - Don't we have a unit test preventing that?

Thanks for the fix. 

Actually I assumed the headersclean test would catch this, but I guess it 
does not try to catch this case. The cmake test will catch this case if the 
cmake test is enabled, and even then only for the headers included by the 
class under test, so not enough coverage.

It might be good to extend headersclean for this case or add a new different 
test for it.

>> 5) Qt3D namespace
>> 
>> This is the first time that all classes in a library are in a namespace.
>> Previously only enums (in various modules) and free functions (in
>> QtConcurrent) have been put in namespaces.
>> 
>> In QtConcurrent, the module name also appears in the header file, but
>> that is not followed by Qt3D libraries.
>> 
>> Given that Qt has never put classes in a namespace like this, is there
>> something to be consistent about here?
>> 
>> 6) QParameter is a very generic name
>> 
>> I realise it is in a namespace, but still...
>> 
>> Qt3DParamter might be better *and* more consistent. Similar applies to
>> other classes.
> 
> It's precisely because of these kinds of issues that we decided to use
> namespaces in Qt3D rather than the poor-man's prefix name spacing.

I don't understand what you wrote. I assume when you wrote 'these kinds of 
issues' you didn't mean that "better *and* more consistent" are the 'kinds 
of issues'. What do you mean?

I would encourage a discussion of why this module needs namespaces when the 
rest of Qt gets by without them. There is certainly a consistency angle. 
Should all new modules use a namespace? Should the namespace correspond to 
the name of the repo the library it's in? Or should there be a namespace per 
library?

> If it's required to not use namespaces to be part of the Qt project then
> we can of course change it. However, I would argue against doing so,
> especially in the light of being able to use some more modern C++ features
> in upcoming QT versions. 

I don't know. As far as I know deciding to use a namespace for this one was 
done without discussion on the mailing list?

I'm encouraging discussion such as on the questions I raised above. 

You're introducing inconsistency, so how will things be made consistent 
again in the future? What is the direction? What is the policy? What is done 
for new modules? What should be done for Qt 6? Everything in different 
namespaces? Or everything in a Qt6:: namespace? Or multiple namespaces? 
Qt6::Core::QString? Do you aim to see 

 using namespace Qt6::Core;
 using namespace Qt6::Gui;
 using namespace Qt6::Network;
 using namespace Qt6::3D;

at the top of most translation units using Qt, as one sometimes sees for 
boost use? Is that the future of what Qt-using code will look like? Do you 
find Qt3D code readable without the Qt3D:: prefix? I don't know as I haven't 
written any. Do you discourage or encourage the use of 

 using namespace Qt3D;

? 

All I know is that we now have inconsistency with this new module.

Then again, if anyone else on this mailing list thinks namespace consistency 
is relevant for Qt, they can also chime in. As they've not done so here, we 
can probably assume the community vibe is that namespace consistency isn't 
something to create or look for in Qt at this time. 

You can start a thread/discussion if you wish :). I'm mostly curious what 
you think this means for the rest of Qt and the future of Qt.

> Name spaces are supported everywhere these days
> so why not just use them, especially in a new add-on module?

I don't know that that was ever the reason Qt doesn't put everything in a 
namespace. If it was we would have changed it for Qt 5.0.

>> I saw one method documented in qparameter.h instead of in the cpp and I
>> didn't look for more.
> 
> The docs need a lot of work. I have some time set aside for doc writing
> this week and will do a clean up pass for such issues.

Cool. Having docs in headers doesn't create API or other release-relevant 
issues anyway.

Thanks,

Steve.





More information about the Development mailing list