2013 m. birželio 19 d., trečiadienis

The dangers of C++ smart pointers

I've spent an entire day debugging a single segfault in Gnote! Luckyly it only occurred "sometimes" when closing application. It was an issue with C++ smart pointer, Glib::RefPtr to be exact. Here are some dangers you might encauter with all types of smart pointers:
  • Mixing smart and simple pointers is never a good idea, SPs only trully work when used entirely everywhere
  • Make sure you know how pointer WORKS, look at it's source code, if required. Assumptions is evel everywhere, but here they can bring the house down (as could have happened in Gnote)

On issue in Gnote
Glib::RefPtr is designed to point to descendants of Glib::Object, but is not limited to them. It requires that object has methods reference() and unreference(), which do reference counting and, if necessairy, delete the object. The problem is, that RefPtr constructor expects object to have a counter of 1 and does not call reference() (probably related to how GTK+ works). However the destructor does call unreference(). So, if you create RefPtr passing an object, that's already referenced by other RefPtr's, objects internal counter is less than actual number of RefPtr pointing to it. As a result when all RefPtr's are destroyed, you might get crash on some later of them with some nice time to chase this bug down :)

2013 m. birželio 9 d., sekmadienis

Programming best practices I disaprove

  • Class imports instead of package imports
In languages where it's applicable, it's usually recommended or even required. It has a point in C++, where less #include's means faster compilation time, however it only makes sense for a large project that takes long to compile.
For languages like Java it makes little sense. Compilation time is the same, all you really get is that you have to put in effort maintaining your imports. While IDE can help you here, it is still and extra click/key press over and over again, it's extra code changes in your VCS and clutter for code reviews. What for? Some bureaucratic claims of nicer code, nothing more.
  • Code-to-interface
This practice requires to define interface and make code depend on it and not on implementing class. Reasons are clear: easy to create second implementation, easier to test (really?), more enforcement on a proper use of class.
The problems occur, when an overuse kicks in. In my view interface is good, when there is more than one implementation of it. In other cases it's just extra piece of code that causes you problems and usually is there for no real reason. Besides, refactoring class to interface and implementation usually is not hard, in fact rather trivial, so why write interface in advance, when you can add it just when the need arises?
  • Banning any language feature
In corporate coding standards you can find various rules like "use of goto is not allowed", "ternary operator is not allowed" etc.
If language has feature that's not deprecated, why someone should not use it? The use must be proper, of course! Even an ugly thing like goto can actually make code more readable and understandable, just you have to use it in right place and in the right way.
  • Setters/getters, no public fields
That's the most Java-way, public fields are generally banned in Java, everyone just codes setters and getters without thinking. It's even enforced by frameworks etc. Each time I see a five year old class with nothing but private fields and public trivial setters and getters for each of them, I wonder why? To write more code? To prepare for change that's unlikely to happen in a lifetime? And if change happens, are you really ready? You haven't declared throws on a setter, so how are going to add potential validation in future? Yes, you can add unchecked exception, but is existing code ready for that?
  • Single return
Some claim that multiple returns make code more complicated. I find it quite the opposite. When function/method has to do some cleanup before exit, then probably using single return makes it simpler, but in other cases it make it more complicated, adds extra if's and nested blocks etc.
  • Separation of concerns as much as possible
The key point here is "as much as possible". Some people take this to extreme, even beyond the line of sanity. It's fine to chop large and complex problems into smaller and simpler ones, but it should not create new problems. When you chop tree into toothpicks, what you get is a crap. Now you put some design patters to put some pieces together, then another patterns to tie those... Eventually you end up with a crap of patterns and start looking for more fancy ways to implement them...
Some things are interdependent, you can't actually separate them, attempt to do this artificially will only cause you more problems.
Some problems are just hard and you can't simplify them by cutting them in peaces.

To end this on a positive note, here are what believe to be universally best practices:

  • Everything you do, do it for a reason
  • If what you did doesn't meet objectives - undo, replace or fix it
  • Throwing junk away is usually the right thing to do, whatever the cost of that junk