Coding practices

Warning, this is a “dear lazyweb post”, however help would be very appreciated. While working on my GSoC project I see a lot differences between code written by various people. When I look at the code I wrote previous GSoC I’m not always very pleased with especially when I compare it with code I see in Akonadi for example. This brought me to a couple of questions:

  1. When should I use a d pointer in stead of private/protected members, and should I always use QSharedDataPointer when using a d-pointer?
  2. Should I use:
    copy( const Record* from, Record* to )
    copy( const Record& from, Record& to )
    and if the last option, why?

Well, if you have some answers for me I would be very pleased =:)

This entry was posted in coding. Bookmark the permalink.

33 Responses to Coding practices

  1. Alexei Sergeev says:

    last. Because in second case you can get null or uninitialized pointers and then you should check them and… user should know if he must create second object by himself or you return new one and and and…
    So use last one.

  2. Alexei Sergeev says:

    I wasn’t clear, for second question you should use references instead of pointers.

  3. kov says:

    But we still need to know the why.

  4. Anonymous says:

    As I said in case you use references user always pass initialized objects as params.
    In case you user pointers user can pass uninitialized pointers by error or he can suppose that second object ( copy to ) will be created by function.

    With references you shouldn’t add any checks to you function.
    With pointers you should at least check if pointers are not null (and throw exception), more than that, if they are not null pointers, they could be still uninitialized and you can’t throw exception or report user about what is wrong.

    With references you could do all the things exactly the same as with pointers, but define by default that user should create objects. So why mess up with pointers at all?

  5. Bertjan says:

    He well, the actual reason was quite stupid. When I started working on that lib I wasn’t very known with C++ programming. Now I know that you need to implement copy constructor and operator= but at that time I didn’t know and didn’t take the time to learn.

    However, now I know and your explanation sounds quite reasonable.

  6. Andreas says:

    If you read code that you don’t know it is often impossible to distinguish in parameters from out parameters. Therefore non-const pointers should be used for out parameters and for in parameters it’s usually const reference or pointer to const.
    In good C++ code it is rare to pass around ownership of anything so it should be clear that the pointer does not mean any change of ownership if not explicitly documented or hinted by the method name (like createFoo()). Qt has it very, very right. I hate non-const references, I really do. They are a first-rate obfuscation technique.

  7. Alexei Sergeev says:

    It is your personal ‘hate that’ thing, but use pointers and references at the same time is even worse practice. Code became unreadable and hard to write.

    void copy_r( const A&, A& );
    void copy_p( const A *, A * );
    void copy_b( const A &, A* );

    // Use objects on stack.
    A a;
    A b;
    copy_r( a, b ); // ok, clear.
    copy_p( &a, &b ); // not very clear, but still ok.
    copy_b( a, &b ); // not clear, not intuitive. (may be for you it is ok)

    // Use objects on heap.
    A *a = new A();
    A *b = new B();
    copy_r (*a, *b ); // not very clear, but still ok.
    copy_p( a, b ); // not good, but pretty clear.
    copy_b( *a, b ); // not clear, not intuitive. (may be for you it is ok)

  8. zkgkckzpgtp says:

    7KqZZ6 hctstmhbgogb, [url=]xxojvuondkzk[/url], [link=]zqpeucqxwvjz[/link],

  9. lkgcdov says:

    CRPoRU jvqrgrlgedyv, [url=]qiblqseyqipv[/url], [link=]qqgldjckfabp[/link],

  10. cedrndxoo says:

    vbR21F mkyufjpyuryd, [url=]zhraulewvupl[/url], [link=]rulrssvxtfjp[/link],

  11. ehzzuzxx says:

    jV7Igc evpyfiusqyys, [url=]omsivcimqujs[/url], [link=]ncaodholckjk[/link],

  12. frinring says:

    To 2.:
    I recommend a third variant:
    copy( Record* to, const Record& from )

    to first, because the order for arguments is best “out” before “in”, so the modified object comes first, then some non-modified parameters.

    And I found it helpful to have modified arguments given as pointers instead of reference, so in the calling code these arguments stay out, due to the address operator & before the variable name (I just wish C++ had a calling syntax similar to Object-C): copy( &target, source );

  13. pfgwmroz says:

    yJwjPq ofwiceqtvtee, [url=]usxwrztvihhl[/url], [link=]dlmqfqwdjhjy[/link],

  14. Sebastian Trueg says:

    * Use d-pointers whenever you have a public class.
    * Better never use protected data members. Expose them through protected methods.
    * Use QSharedDataPointer for struct-like classes that you want to copy around a lot (QString is the typical example). Using it with QObjects does not make any sense as those cannot be copied anyway.

    As for the copy:
    Instead of the reference variant use proper copy operators and constructors (those can be perfectly combined with QSharedDataPointer in case Record is a struct-like class). There are only two reasons I can see for a copy method:
    1. Copying the contents of a QObject (again: QObject cannot be copied the C++-way)
    2. a class hierarchy where you use some visitor-like pattern or something. In this case you use pointers because it is way easier to read.

    Hope this helps.

  15. JSabatier says:

    The difference between & and * is the contract between the caller and the called function.

    By using a & the contract of the caller is to provide you a good object. By using a *, this can be a null pointer, or a pointer on a bad address…

    In fact you have to use reference as often as you can (like const).

    copy( const Record& from, Record& to ) shouldn’t be copy( const Record& from, Record& to ) const; ?

    I recommend you to read Scott Meyers book for C++ programming and Bertrand Meyer for design by contract.

  16. rrsrkiezk says:

    6RcXmc wlivsakkgwzt, [url=]dlshjjgvoqba[/url], [link=]rbzzdvsfwznw[/link],

  17. Tristan says:

    The maxim “use references when you can, and pointers when you have to” is best in almost every case.

  18. nnieibwp says:

    8e4cTC fpydekusfmxq, [url=]jcisrkqmdadd[/url], [link=]gtdpocyraxuu[/link],

  19. Ian Monroe says:

    d-pointers are annoying and I’d wish people wouldn’t use them in non-library code. They’ve invaded Amarok a bit, supposedly with the purpose of making it possible to add data members without triggering recompiles, but I don’t buy it. I think it sacrifices too much readability.

    So if your not developing a library, the answer to the question “should I always use QSharedDataPointer when using a d-pointer” is “yes” in my opinion: its the only legitimate reason to use a d-pointer in application code.

  20. Bertjan says:

    The sacrifice of readability is mostly a matter of taste I think. When you use only one letter for the variable then only “d->” is added at places where you otherwise would read only the member name. I for sure found it much more a pleasure to read the Akonadi code with d pointers than my own code. But the absence of d pointers might not be the only reason =:D.

    Anyway, still enough to learn about writing beautiful code.

  21. Ian Monroe says:

    Suppose I mostly just don’t like rutting around the .cpp file trying to find the private class declaration so I can see what the variable is declared as. .h files do have the one redeeming quality of being nice to look stuff up in.

    If I worked at with it enough, perhaps I could start to ignore d-> and see it as no different then m_, but currently they throw up “aaah, pointer dereference, what if its null or dangling” even though this isn’t ever a problem with d pointers. :) So they look quite ugly to me.

  22. Bertjan says:

    In Akonadi they solved this by moving the class declarations of private classes to seperate header files.

    class Item -> item.h, has member ItemPrivate -> item_p.h

    This is a method I really like. The code of the akonadi libraries is very easy to read if you ask me.

  23. Jeff Mitchell says:

    In Akonadi, and all of kdelibs :-)

  24. jgnjublrbiy says:

    ujpn2c kkalqwnazflp, [url=]mcbgdiforvhs[/url], [link=]dicitcyxvira[/link],

  25. irjzuzvuqmo says:

    QbONuu fpfhqbssooea, [url=]rktfenyoueyk[/url], [link=]cvdvvcxubnvy[/link],

  26. Alex says:

    For the copy(): use pointers, never use non-const references.
    When you read the calling code, you don’t get a hint that the value of an argument might be modified by the called function if it is called with a reference. If it is a pointer, there can be only two reasons for that:
    -it is actually an array
    -the called function modifies the variable
    So if the argument is const, use a reference or pointer. If it is non-const, use a pointer.
    Check out “Designing Qt-Style C++ APIs” by Matthias Ettrich:

    Regarding pointers vs. references:
    A pointer can be NULL, a reference can’t be something similar. This can be both an advantage and a disadvantage depending on what you want to do (see QWidget: if the parent is NULL, it’s a top level widget, if it’s not NULL, it’s a child widget, this works nicely with pointers).
    Additionally, a pointer can point to something which doesn’t exist anymore. The same is true for references, they can also reference something which doesn’t exist anymore. In both cases you can’t test for it:

    int main()
    int i=17;
    int& ir=i;
    int j=3;
    // at this point ir references a non-existing variable
    return 0;


  27. Bertjan says:

    That links goes directly into my bookmarks. Thanks for the very clear comment!

  28. JSabatier says:

    Yes sure, you can even do something like that:

    {sabatier.xxxx}[160] cat *.cpp && g++ *.cpp && ./a.out
    #include using namespace std;
    char FuckingFunction(int &FuckingReference = *static_cast(NULL)) {
    return &FuckingReference?FuckingReference+’0′:’X';

    int main(int uArgs, char **pArgs) {
    cout < < "yEp dah Fuckoff fucking rEfereNCe" << endl;
    cout << "yOp what that? -> ” < < FuckingFunction(++uArgs) << endl;
    cout << "yOp what that? -> ” < < FuckingFunction() << endl;
    return 0;
    yEp dah Fuckoff fucking rEfereNCe
    yOp what that? -> 2
    yOp what that? -> X

    But what you forgot is that you made a bad code. With bad code you can prove everything…
    I can provide you something in contradiction found in… C++ language:
    A &operator=(const A &);

    Please read scott meyers… a real good reference in C++

  29. berkus says:

    And code in this comment is itself a very good example of how NOT to write your code :)

  30. Kevin Kofler says:

    This is invalid code, NULL references are not allowed in C++ and using them invokes undefined behavior. And invoking undefined behavior means the compiler can output code doing anything, even formatting your hard disk.

  31. wbiain says:

    1yAxz7 qzkxoqabaxey, [url=]ydrbygxnjaze[/url], [link=]plmrqopcydbf[/link],

  32. randomguy3 says:

    d pointers:

    d pointers are needed (or, at least, very much recommended) if you need to maintain binary compatibility for any reason, such as a public class in a library. This is the reason kdelibs has them all over, and Akonadi uses them.

    However, for internal classes, I wouldn’t bother unless it’s an implicitly shared class (using QSharedDataPointer, for example). Why? Well, you have to manage the pointer (one of the most common causes of memory leaks in KDE I’ve seen is people not delete d-pointers). Also, if you use a sane initialisation method (a constructor initialises an object completely as far as possible, including setting unused pointers to 0 and other variables to a sensible default), the inititialisation of members used in class Foo takes place in class FooPrivate, which is not necessarily where you’d look.

    However, if you don’t use d-pointers, I’d use the convention of starting member names with m_, since that makes it obvious that something is a member and not, say, a function-local variable in the code and also provides a distinct namespace for members and methods. This is often important for variables with getters and setters, because you often want to call the getter and the variable the same thing.

  33. Kevin Kofler says:

    Qt style code would be copy( const Record& from, Record* to ): use references where the object is constant, pointers when you’re writing to it. Rationale: passing a const reference is just an optimization, the caller does not need to know, but when the object is going to be modified, the only way to make this obvious is to use a pointer.


Comments are closed.

GSoC Update: Finished category syncing and Contacts conduit

Things are progressing very nicely although not at the speed I original intended. Yesterday I finished some small bugs in the Category sync code which were still there when doing a life sync. Now you can actually add a category to you palm and it will be added at the pc side during sync and vice versa. The cool thing of this is that this code is in the base library which means that all conduits based on it will have support for category syncing. The fixes make the KeyringConduit less or more fully functional. The only problem is that there is not really a good Keyring database editor. And no don’t try the java keyring editor as it will bork your database file. As I needed something for testing I started working on a editor but that one is far from complete and I first need to finish my GSoC work. It does, however, support adding, deleting and modifications of records as well as creating new and opening existing databases.

Another thing that I’ve finished ™ the last couple of days is the contacts conduit. This conduit is able to sync the addressbook of the palm with an Akonadi contacts resource (currently I only tested with the vcard resource, but I guess other resources that only contain contacts should work as well). Of course there will be some polishing and bullet-proofing needed before it is ready for day-to-day use but the first results are really promissing. I really have to say that working with the Akonadi libraries is a real pleasure. Good work guys!

Next steps will be abstracting some code that can be used for all conduits that are talking to Akonadi and port the other two PIM conduits Calendar and Todo.

Oh and before I forget: which is only possible thanks to generous sponsoring from the KDE e.v.

This entry was posted in coding. Bookmark the permalink.

2 Responses to GSoC Update: Finished category syncing and Contacts conduit

  1. Christoph says:

    JPilot has a nice plugin for editing keyring databases

  2. rvvgvcgk says:

    iBMDXL ccypvsixbxsh, [url=]gmnrthjdbgac[/url], [link=]frmxoxfxkocc[/link],

Comments are closed.