[Qt-creator] Couple of questions about the design of Qt Creator

[Elvis Stansvik]

2017-09-12 13:22 GMT+02:00 Eike Ziller <Eike.Ziller at qt.io>:
>
>> On 12. Sep 2017, at 11:42, Elvis Stansvik <elvstone at gmail.com> wrote:
>>
>> 2017-09-11 9:50 GMT+02:00 Eike Ziller <Eike.Ziller at qt.io>:
>>>
>>>> On Sep 11, 2017, at 09:24, Elvis Stansvik <elvstone at gmail.com> wrote:
>>>>
>>>> 2017-09-10 11:31 GMT+02:00 Elvis Stansvik <elvstone at gmail.com>:
>>>>> 2017-09-10 11:03 GMT+02:00 Elvis Stansvik <elvstone at gmail.com>:
>>>>>> Hi all,
>>>>>>
>>>>>> In a quest to find inspiration for good Qt application architectures,
>>>>>> I've been looking at the plugin based one you're using in Qt Creator.
>>>>>> It strikes me as a really nice design.
>>>>>>
>>>>>> I've been reading the available docs on it, and dug into the code a
>>>>>> bit. This may be a bit much to ask, but I was wondering if any of you
>>>>>> devs could answer a few questions that popped up? It would be much
>>>>>> appreciated!
>>>>>>
>>>>>> It's really just two questions, about two different topics:
>>>>>>
>>>>>> 1. The Invoker / invoke<...> Thingie:
>>>>>>
>>>>>> You have ExtensionSystem::Invoker and the associated invoke<..>
>>>>>> helper, which are syntactic sugar for achieving "soft" extension
>>>>>> points. It seems it's not used that much (?). I grepped for
>>>>>> "Invoker|invoke<" in the code and could only find a few uses of it. I
>>>>>> also grepped for "invokeMethod" to see if the approach was being used
>>>>>> "manually" so to speak (without the sugar), and found a few more hits.
>>>>>>
>>>>>> What was the motivation for adding this? I assume it's for cases where
>>>>>> you want a looser coupling between plugins (no linking, no shared
>>>>>> header), but can you give an example of when you really wanted that
>>>>>> looser coupling and why?
>>>>>>
>>>>>> 2. The Plugin System in General:
>>>>>>
>>>>>> Is there anything about the plugin system in its current form, or how
>>>>>> it is used, that you would do fundamentally different if you could do
>>>>>> it all over again? Any areas that you find messy/awkward, that need a
>>>>>> re-think/makeover? In short: What are the biggest warts in the code in
>>>>>> your opinion?
>>>>>
>>>>> As soon as I hit send, I realized I have a third question:
>>>>>
>>>>> 3. Communication Between Plugins:
>>>>>
>>>>> There seems to be two main mechanisms through which plugins
>>>>> communicate: Either objects that implement shared interfaces are added
>>>>> to the plugin manager object pool and picked up by downstream or
>>>>> upstream plugins (in the top-down or bottom-up phase of plugin
>>>>> initialization, respectively), or a singleton instance is acquired and
>>>>> calls made on it.
>>>>>
>>>>> Is the former approach used when dependants provide functionality to
>>>>> their dependees (which are unknown), and the latter approach used when
>>>>> dependees use their dependants (which are known)? Is that the deciding
>>>>> factor?
>>>>
>>>> And finally, a couple of more down-to-earth questions:
>>>>
>>>> 1. ICore, the class is concrete, so why the I in the name? Was it
>>>> abstract at one point?
>>>
>>> Yes historically.
>>>
>>>> How do you decide whether a class should get
>>>> the interface 'I' in its name?
>>>
>>> It’s a mess ;)
>>> I suppose the trend goes to not prepend the ‘I’.
>>>
>>>> The same with e.g. IContext, though
>>>> that one at least has a few virtuals and is used as a base class (but
>>>> no pure ones AFAICS, so still concrete).
>>>
>>> Historically these classes where “pure” virtual (except for the QObject base).
>>> We moved to a more “configurable” approach then to avoid the need to create subclasses for every little thing, while keeping the option open in many cases.
>>>
>>>> 2. The relatively liberal use of singleton classes. We all know that
>>>> is a debated subject, and I don't have an opinion either way. I'm just
>>>> interested in if you have some (spoken or unspoken) policy regarding
>>>> singletons in the project. Do you want to minimize the use of them, or
>>>> is it OK for newer code, or is it judged on a case-by-case basis? Have
>>>> you had any moments where you really wish you hadn't used singletons?
>>>> (e.g. I know it can sometimes hurt testability).
>>>
>>> We always had a liberal amount of singletons in Qt Creator, and we even moved most of them to be classes with mostly static methods a while ago. There are no plans to move away from that. If you have a central hub for “managing” something, feel free to use a singleton/static methods.
>>
>> Just a small additional question regarding singletons.
>>
>> For example, in PluginManager, you have the following setup:
>>
>> namespace ExtensionSystem {
>>
>> static Internal::PluginManagerPrivate *d = 0;
>> static PluginManager *m_instance = 0;
>>
>> /*!
>>    Gets the unique plugin manager instance.
>> */
>> PluginManager *PluginManager::instance()
>> {
>>    return m_instance;
>> }
>>
>> /*!
>>    Creates a plugin manager. Should be done only once per application.
>> */
>> PluginManager::PluginManager()
>> {
>>    m_instance = this;
>>    d = new PluginManagerPrivate(this);
>> }
>>
>> ...
>> }
>>
>> That is, instead of the normal singleton pattern (private constructor,
>> lazy construction in instance()), the singleton aspect is by
>> agreement.
>
> - Control over the lifecycle. There is some code responsible for creating the singleton, which is also responsible to destruct it in a controlled and defined manner.
> - Less checking. Since the lifecycle is defined, the (mostly static) methods do not need to ensure existence of the singleton. They directly access e.g. m_instance.
>
> In many cases the constructor could be private with friend declaration for the code constructing the instance (but not in the PluginManager case, which is API in a separate library, and I’m no big fan of friend either).

Alright, thanks for clarifying. I was just a little unused to seeing
it done like this, but it makes sense.

>
>> Also, the m_instance and d-pointer are not kept as members of the
>> class, but globally in the enclosing namespace.
>
> They are static in the .cpp, so only local.
> There is just no need for them to be members of the public class.
>
> Of course all of this is also a matter of taste and socialization ;)

Ah right, it's actually a form of hiding. Didn't think of it that way,
but now I understand Andrés reply as well.

Thanks to both of you.

Elvis

>
> Br, Eike
>
>>
>> What is the reason for this approach?
>>
>> Elvis
>>
>>>
>>> Br, Eike
>>>
>>> --
>>> Eike Ziller
>>> Principal Software Engineer
>>>
>>> The Qt Company GmbH
>>> Rudower Chaussee 13
>>> D-12489 Berlin
>>> eike.ziller at qt.io
>>> http://qt.io
>>> Geschäftsführer: Mika Pälsi,
>>> Juha Varelius, Mika Harjuaho
>>> Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B
>