Some issues

[mitchcurtis]

I understand that coding conventions can be quite subjective (and that a lot of these conventions came from Qt's official docs), but regardless, here are some observations I made while reading this, based on my experience as a developer of and with Qt who writes a lot of QML.

Most of it looks good to me, so I'm just pointing out the stuff that stuck out.

Property Ordering

https://github.com/Furkanzmc/QML-Coding-Guide#property-ordering says:

If you want to declare custom properties for a component, the declarations are always above the first property assignment.

I understand that this is from https://doc.qt.io/qt-5/qml-codingconventions.html#qml-object-declarations, but I think our own conventions are wrong (or outdated) here. The order that I use is:

• id first
• bindings to properties, starting with the oldest ancestor base type and ending with the closest base type
• property declarations
• signal handlers
• attached properties
• function declarations
• large property assignments (like model and delegate)
• child items

Personally I find it very jarring to declare new properties before existing ones are assigned to. :D

Function Ordering

https://github.com/Furkanzmc/QML-Coding-Guide#function-ordering says:

Public function implementations are always put at the very bottom of the file.

"very bottom of the file" is perhaps a bit too far. Personally I would say after attached properties, as above.

Animations

https://github.com/Furkanzmc/QML-Coding-Guide#animations says:

When using any subclass of Animation, especially nested ones like SequentialAnimation, try to keep the Animation objects to one line for readability. You will benefit from keeping the animations as simple as possible since they are executed every frame, and also find it easier to see the execution of the animation in your head.

This is not a good argument for bunching up code onto one line. I know there are people (Qt developers, too) who will disagree with me on this, but I think any more than 2 property bindings on one line is too much. I think it makes it less readable, actually.

A code comment says:

Good. This is easier to read as your eyes find it easy to detect things horizontally rather than vertically.

I wholeheartedly disagree. :D

Giving Components ids

https://github.com/Furkanzmc/QML-Coding-Guide#giving-components-ids says:

If a component does not need to be accessed for a functionality, avoid setting the id property. This way you don't clutter the namespace with unused ids and you'll be less likely to run into duplicate id problem.

In one particular case this is very good advice: if you assign an id to a delegate in a Control (Qt Quick Controls 2), it will prevent deferred execution of that item, which is bad, for performance reasons. However, it's probably not worth mentioning here, and I plan to document it in the Qt Quick Controls 2 docs soon.

I would also add that ids can be useful for identifying what an item is/does, though I would strongly encourage setting an objectName in that case, as it's very useful for debugging and testing.

It is a good idea to use max 3-4 character abbreviation for the ids so that when you are looking for a certain component, say a TextBox, it will be easier to list the IDs of all the text boxes by just typing tb.

Sorry, but is not something I would consider a best practice. Names should be descriptive, and if that means making them long, so be it. This becomes so important when you have to maintain an existing codebase; if a variable is not well-named, it makes everything just that little bit harder. A good IDE solves the problem of typing with auto-completion.

Make sure that the top most component in the file always has root as its id.

This is something I can get behind though. It is one less thing that you have to think about when writing QML code, and I imagine it will become even more relevant when the QML engine devs start making scope lookup more strict:

• https://bugreports.qt.io/browse/QTBUG-76016
• https://bugreports.qt.io/browse/QTBUG-71578 - specifically "unqualified lookup in the root scope of a component"

Property Assignments

https://github.com/Furkanzmc/QML-Coding-Guide#property-assignments says:

When assigning grouped properties, always prefer the dot notation If you are only altering just one property. Otherwise, always use the group notation.

Personally I've never bothered with this. If there's a performance reason for it, it would be worth mentioning as justification.

Import Statements

https://github.com/Furkanzmc/QML-Coding-Guide#import-statements says:

Imports take time in QML. And If you are developing for a device with low system specifications, then you will want to optimize as much as possible. In that case, try to minimize the number of imports you use in your QML file.

I don't like this advice, because it conflicts with the idea of keeping QML files as small and self-contained as possible. If users follow this advice, they will end up with gigantic QML files. As for the performance side of it, I have no idea if it's true or not, but that's something Qt should aim to fix if it's an issue, rather than advocate for less imports. :)

https://github.com/Furkanzmc/QML-Coding-Guide#import-order says:

When importing other modules, use the following order;

• Qt modules
• Third party modules
• Local C++ module imports
• QML folder imports

Agreed! Using a similar order to C++ is what I do.

Item 2: Bindings

https://github.com/Furkanzmc/QML-Coding-Guide#item-2-bindings says:

Bindings are evaluated whenever a property it depends on changes and this may result in poor performance or unexpected behaviors.

Technically true, but a bit fear-mongerish in my opinion. :)

Even when the binding is simple, its consequence can be expensive. For instance, a binding can cause the position of an item to change and every other item that depends on the position of that item or is anchored to it will also update its position.

True, but also not a huge deal in most cases.

So consider the following rules when you are using bindings.

Reduce the Number of Bindings

The premise here is wrong, I think. Trying to improve performance by replacing declarative bindings with imperative assignments is backwards, and will likely result in slower code for all but the most extreme cases (i.e. expensive bindings). We have tools like the QML profiler to address bottlenecks in applications, so we should instead recommend that that be used if the user suspects their bindings are slow. When they've confirmed the binding is getting re-evaluated too often, and they've verified that there's nothing that can be improved in any relevant C++ code, then perhaps they could use profile-guided optimisation to replace bindings with assignments.

See also: https://doc-snapshots.qt.io/qt5-5.13/qtquick-bestpractices.html#prefer-declarative-bindings-over-imperative-assignments

WIP

[Furkanzmc]

Property Ordering

• id first
• bindings to properties, starting with the oldest ancestor base type and ending with the closest base type
• property declarations
• signal handlers
• attached properties
• function declarations
• large property assignments (like model and delegate)
• child items

I think property assignments should be as close as possible. When a file gets bigger, it gets hard to tell whose property I'm assigning to because it's hard to discern the indentation and which item that level refers to.

I find that code gets harder to read when you go from assignments to functions and to assignments again.

Personally I find it very jarring to declare new properties before existing ones are assigned to. :D

And the way I see it it lets me know at the top that these are the properties and I can assign to them like I would with the built-in properties. And also it's part of the interface, and when I look at a QML file, I see them first and I know what properties are available.

You could argue the functions are also part of the interface and they should be at the top, but I try to use as few functions as possible. And with functions, I see them as things that operate on the entirety of the component. So, when they are down below I mentally find it easy to understand the relationship to the object it's declared in.

Animations

This is not a good argument for bunching up code onto one line. I know there are people (Qt developers, too) who will disagree with me on this, but I think any more than 2 property bindings > on one line is too much. I think it makes it less readable, actually.

I agree with you. When I initially wrote this, I was fine with it. But as time went on, and I worked more with QML and on bigger projects, multi line is almost always better than single line. I usually find it acceptable for one line if it doesn't exceed 80 characters.

Giving Components IDs

Sorry, but is not something I would consider a best practice. Names should be descriptive, and if that means making them long, so be it. This becomes so important when you have to maintain an existing codebase; if a variable is not well-named, it makes everything just that little bit harder. A good IDE solves the problem of typing with auto-completion.

You are right. I no longer follow that practice. I should have updated it. I did not know better when I wrote this.

Import Statements

I don't like this advice, because it conflicts with the idea of keeping QML files as small and self-contained as possible. If users follow this advice, they will end up with gigantic QML files. As for the performance side of it, I have no idea if it's true or not, but that's something Qt should aim to fix if it's an issue, rather than advocate for less imports. :)

When I wrote this, I did a profiling and initially QML imports were taking a while. I believe this was with 5.11. So I should do it again to confirm If that's the case or not. Also, knowing that some people use QML for embedded development as well, I wanted to point out some cases that might affect performance. I think it should be more clear so that people don't go the premature optimization way.

Item 2: Bindings

The premise here is wrong, I think.

You are absolutely right. That section needs to be removed. I can maybe change it so that it can say don't repeatedly update a property that other's are bound to. So instead of;

for (let i = 0; i < 100; i++) {
    root.someProperty += i
}

They do:

let tmp = root.someProperty
for (let i = 0; i < 100; i++) {
    tmp += i
}

root.someProperty = tmp

---

Thanks a lot, Mitch! I'm looking forward to more of your feedback.