Make member variable names, & placement and * placement more coherent
ClosedPublic

Authored by jeanv on Aug 14 2017, 4:50 PM.

Details

Summary

Change all member variables to the form m_fooBar because it is the preferred form in Qt (it was half and half between this and mFooBar, and a minority didn't have anything).
Place all references and pointers on the side of the type since it is the majority.

Basically:

  • mFoo -> m_foo
  • foo -> m_foo (if it is a member variable)
  • Type &ref -> Type& ref
  • Type *ptr -> Type* ptr

Diff Detail

Repository
R224 KDE Connect
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jeanv created this revision.Aug 14 2017, 4:50 PM
jeanv updated this revision to Diff 18167.EditedAug 15 2017, 8:48 AM

Include double pointers (Type **foo -> Type** foo)

nicolasfella accepted this revision.Aug 15 2017, 10:16 PM
This revision is now accepted and ready to land.Aug 15 2017, 10:16 PM
albertvaka requested changes to this revision.Aug 16 2017, 1:20 PM
albertvaka added a subscriber: albertvaka.

I see some inconsistencies, for example in DaemonPrivate you changed
QSet<LinkProvider*> mLinkProviders; to
QSet<LinkProvider*> m_linkProviders;

while in KdeConnectPluginPrivate you changed
Device* mDevice; to
Device* device;

Also, I see you renamed d-pointers (https://wiki.qt.io/D-Pointer) like QScopedPointer<KdeConnectPluginPrivate> d; to QScopedPointer<KdeConnectPluginPrivate> m_pluginInfos; and we should keep the naming so it is clear we are using this programming pattern.

This revision now requires changes to proceed.Aug 16 2017, 1:20 PM
jeanv added a comment.Aug 16 2017, 4:34 PM

I see some inconsistencies, for example in DaemonPrivate you changed
QSet<LinkProvider*> mLinkProviders; to

`QSet<LinkProvider*> m_linkProviders;`

while in KdeConnectPluginPrivate you changed

`Device* mDevice;` to
`Device* device; `

Also, I see you renamed d-pointers (https://wiki.qt.io/D-Pointer) like QScopedPointer<KdeConnectPluginPrivate> d; to QScopedPointer<KdeConnectPluginPrivate> m_pluginInfos; and we should keep the naming so it is clear we are using this programming pattern.

I did not know about d-pointers, that's my bad :). I'll revert the names and make all member variables inside the structures coherent with the rest.

What do you think about naming them d_ptr like in the Qt doc? It would be more explicit to newcomers that this is deliberate.

In D7312#136390, @jeanv wrote:

What do you think about naming them d_ptr like in the Qt doc? It would be more explicit to newcomers that this is deliberate.

In most KDE projects they are just called 'd', and I think that adding '_ptr' won't make it that more clear, but do as as you wish :)

jeanv added a comment.Aug 17 2017, 6:54 PM
In D7312#136390, @jeanv wrote:

What do you think about naming them d_ptr like in the Qt doc? It would be more explicit to newcomers that this is deliberate.

In most KDE projects they are just called 'd', and I think that adding '_ptr' won't make it that more clear, but do as as you wish :)

No problem, I'll leave it as it is.

jeanv updated this revision to Diff 18304.Aug 17 2017, 6:57 PM
jeanv edited edge metadata.

Reverted the erroneous renaming of d-pointers. And properly renamed a few missing member variables

jeanv added a comment.Sep 1 2017, 1:32 PM

So, is the revision good?

In D7312#142075, @jeanv wrote:

So, is the revision good?

Yes, sorry, I will push it now.

albertvaka accepted this revision as: KDE Connect.Sep 3 2017, 7:42 PM
albertvaka accepted this revision.
This revision is now accepted and ready to land.Sep 3 2017, 7:42 PM
This revision was automatically updated to reflect the committed changes.