Monday, November 1, 2010

Temporary objects

Most believe that temporary objects are const, well they are "almost const". It is possible indeed call on a temporary object a non const member. Nifty exception stated in the standard: "Section 3.10.10 in C++ ISO/IEC 14882:1998".
This exception permits to implement a movable constructor (through a proxy) while waiting for Rvalue references in C++0x language standard.





Wednesday, August 25, 2010

Friday, February 12, 2010

Asserts vs Exceptions

I still see people with the doubt: "shall I use an assert or throw an exception?".
Let me whine a bit about the usage of asserts first. Overall people code badly and I mean it, hence assertions are not used enough. Having say that, "assert" and "exception" have different
meanings and behaves.

A failure assert just terminates the normal program execution (reporting source file name and line position where the error occurred), an exception thrown has a chance to be caught. Asserts disappear/vanish when NDEBUG is defined (usually in production code); this fact is a newbie oversight and what they claim about the massive usage of assert is the overhead introduced by asserts. If I wasn't clear: "Asserts disappear in production code". What didn't you get from: "Asserts disappear in production code"?

Those two facts already give an hint on when to use an assert and when an exception. If the error can be managed (by user or code itself) then an exception must be used, if the error has no chance to be managed by anyone then assert is your friend. This is not the only rule to follow indeed as said: "Asserts disappear in production code" and then in production code that error will not be "detected", errors that you expect to happen in production code can not be spotted by an assert.
Asserts are meant to detect coding error and there is no reason to happen in production code. When writing a method there are some assumption about the internal class status (pre-condition), better to check those first in order to not do something bad, at the end of the same method better check that the internal status of class is in a consistent status (post-condition).
Unfortunately, there are a great deal of people that are using exceptions for case that really ought to be assertions. Throwing exceptions instead of wrapping simple pre/post-conditions into a simple assertion macro is an hint of the fact that you're coding badly. Asserts are used to protect by coding error: using a null pointer for example; exception are to protect by normal program life: server not available, file not writable, etc.

It should be always possible to write a unit test that is able to make an exception to be thrown, if you are not able to then it means that that piece of code is a dead code; to the other side it should be impossible do the same with assert, if you are able to write an unit test that is able to make an assert fail then most likely that assert should be an exception, or of course the code is wrong.

A nice rule that drives the lazy bone programmers to use more assert is: who ever write a code that provides a set of data that asserted then the same person is responsible to fix it. This rule drives library writer to protect them self by an incorrect usage of their library at the cost to fix even the code using the library.

If you have missed it: "Asserts disappear in production code".

Tuesday, February 3, 2009

Monday, December 29, 2008

Language programming benchmark

I don't want to start a flame-ware about which language is better, I'm just reporting what I found today during some research. The site language benchmark shows the result of some benchmark using different hardware architecture, operating systems, language, language implementation. I was astonished by some facts:
  • C++ (gnu/intel) implementation performs better than the corresponding C implementation both in time and memory usage
  • Intel C++ implementation out perform C++ GNU implementation, both in time and memory usage
  • GNU C implementation out perform Intel C implementation, both in time and memory usage
I don't report cross comparison between C++/C and other languages, you can do it yourself...

Thursday, September 18, 2008

Assignment operator

Today I went through a piece of code similar to this (what matters here is how the assignment operator was written):


class Test {
Test()
:theId(0), theName()
{}

Test& operator=(const Test& aRhs) {
theID = aRhs.theID;
theName = aRhs.theName;

return *this;
}

private:
int theID;
std::string theName;

};

as it is, the code works and I have nothing to say at first glance. However , who knows how the class will be extended in the future?

Imagine that in a future version some members are added:

class Test {
Test()
:theId(0), theName(), theHammer(), theHeap(0)
{}

Test& operator=(const Test& aRhs) {
theID = aRhs.theID;
theName = aRhs.theName;
theHammer = aRhs.theHammer; //This is going to be a bottleneck
theHeap = aRhs.theHeap; //This is wrong

return *this;
}

private:
int theID;
std::string theName;
HugeClass theHammer; //Extra member with huge footprint
HeapClass* theHeap; //Extra member on heap
};
as you can see with these two extra members following the same code line as the previous version the code becomes invalid. The objection I get is: "if one day someone adds those two members then they will take care to write it correctly", unfortunately in the real world it doesn't work like this. The average programmer will just add those two extra members following the code line already in place, following the rule: "after all if the code works for the already present members why shouldn't it be the same for the two extra members I have been told to add?"

The trick to avoid problems like this is to write the right code, from the start thinking of what will happen in the future, when possible of course. Usually the change to make is just a matter of a few lines of code and a two line comment in order to warn the future coders.

The original well written class would have been:

class Test {
Test()
:theId(0), theName()
{}

Test& operator=(const Test& aRhs) {

//Check for a self assignment
if (this != &aRhs) {
theID = aRhs.theID;
theName = aRhs.theName;
}

return *this;
}
private:
int theID;
std::string theName;
};

At this point the objection is: why check for a self assignment when the event is never going to happen? Who will ever write:

Test t;

t=t;
well, the code is valid so be sure someone will, also it is not always easy to spot a self assignment, consider this:

t[j] = t[i];

or even:

Test t;
Test &a = t;
...
t = a;

The real question here is: "Why was the assignment operator in that class was ever implemented?" The question makes a point, the operator was not needed at all, in that case the right thing to do is to remove it.

Let's then suppose the class is the one with the two extra members, the class is managing dynamically allocated memory then the operator must be implemented. The almost correct version is:



class Test {
Test()
:theId(0), theName(), theHammer(), theHeap(new HeapClass)
{}

Test& operator=(const Test& aRhs) {
if (this != &aRhs) {
theID = aRhs.theID;
theName = aRhs.theName;
theHammer = aRhs.theHammer;

delete theHeap;
theHeap = new HeapClass(*aRhs.theHeap);
}

return *this;
}

private:
int theID;
std::string theName;
HugeClass theHammer; //Extra member with huge footprint
HeapClass* theHeap; //Extra member on heap
};


I wrote "almost correct" because the assignment operator is correct but not exception safe, imagine what will happen if an exception is thrown, if that is the case then the class will be left in an inconsistent state. The goal is not an easy one, in order to make an assignment operator exception safe before modifying the internal state it is better to create a temporary object and then swap it with the internal state with operations that do not throw exceptions. This is achieved using the Pimpl idiom, moving all the internal state of Test inside another class and then leaving inside the class Test a pointer to this new class, it is better to use a boost::shared_ptr in this case to avoid exception catches:



class TestImpl {

friend class Test;

private:
TestImpl()
:theID(0), theName(), theHammer(), theHeap(new HeapClass)
{}

TestImpl(const TestImpl& aRhs)
:theId(aRhs.theID), theName(aRhs.theName),
theHammer(aRhs.theHammer), theHeap(new HeapClass(aRhs.theHeap))
{}

int theID;
std::string theName;
HugeClass theHammer; //Extra member with huge footprint
HeapClass* theHeap; //Extra member on heap
};

class Test {

Test()
:theImplementation(new TestImpl)
{}

Test& operator=(const Test& aRhs) {
if (this != &aRhs) {
boost::shared_ptr<TestImpl> tmp(new TestImpl(*aRhs.theImplementation));

std::swap(theImplementation, tmp);
}

return *this;
}

private:
boost::shared_ptr<TestImpl> theImplementation;
};

As you can see writing right code is not easy, and this becomes more difficult if you want to write exception safe code. What I suggest is, to remove assignment operator and copy constructor and write those only if really needed:


class Test {

Test()
:theID(0), theName(), theHammer(), theHeap(new HeapClass)
{}

private:
Test& operator=(const Test& aRhs); //disabled (do not even implement it)
Test(const Test& aRhs); //disabled (do not even implement it)


int theID;
std::string theName;
HugeClass theHammer; //Extra member with huge footprint
HeapClass* theHeap; //Extra member on heap
};

Thursday, June 19, 2008

Threading mess (2)!

I got some comments about my last post about the fact that the showed solution was just detecting the scenario of two or more threads entering at the same time a critical section (in the last post example the method Shared::foo). What it doesn't answer is the real question: "is this class during its life being used by more than a single thread, or more specifically, is a certain section of code used by more than a thread"? Indeed if you remember after a SCOPED_LOCK leaves his scope, it "releases" the stored current ID thread allowing another thread to enter it.

I have added a nested Watch class to ThreadCollisionWarning class that detects also if a critical section is ever used by two different threads ( for example you can detect if a given class is constructed and destroyed within the same thread).

The code is the following:


#ifndef THREAD_COLLISION_WARNING
#define THREAD_COLLISION_WARNING

#include <stdexcept>

#ifdef NDEBUG

#define THREAD_WATCH(obj)
#define SCOPED_WATCH(obj)
#define WATCH(obj)

#else

#define THREAD_WATCH(obj) ThreadCollisionWarning _##obj;
#define SCOPED_WATCH(obj) ThreadCollisionWarning::ScopedWatch scoped_watch_##obj(_##obj);
#define WATCH(obj) ThreadCollisionWarning::Watch watch_##obj(_##obj);

#endif

class ThreadCollisionWarning {
public:
ThreadCollisionWarning()
:theActiveThread(0)
{ }

~ThreadCollisionWarning() { }

class Watch {
public:
Watch(ThreadCollisionWarning& aTCW)
:theWarner(aTCW)
{ theWarner.enter_self(); }

~Watch() { }

private:
ThreadCollisionWarning& theWarner;
};

class ScopedWatch {
public:
ScopedWatch(ThreadCollisionWarning& aTCW)
:theWarner(aTCW)
{ theWarner.enter(); }

~ScopedWatch() { theWarner.leave(); }

private:
ThreadCollisionWarning& theWarner;
};

private:

void enter_self() {
//If the active thread is 0 then I'll write the current thread ID
//if two or more threads arrive here only one will success to write on theActiveThread
//the current thread ID
if (! __sync_bool_compare_and_swap(&theActiveThread, 0, pthread_self())) {

//Last chance! may be is the thread itself calling from a critical section
//another critical section
if (!__sync_bool_compare_and_swap(&theActiveThread, pthread_self(), theActiveThread)) {
throw std::runtime_error("Thread Collision");
}
}
}

void enter() {
if (!__sync_bool_compare_and_swap(&theActiveThread, 0, pthread_self())) {
//gotcha! another thread is trying to use the same class
throw std::runtime_error("Thread Collision");
}
}

void leave() {
__sync_fetch_and_xor(&theActiveThread, theActiveThread);
}

pthread_t theActiveThread;
};

#endif



The nested Watch class (used by WATCH macro) just during his constructor initializes theActiveThread member with the current id thread if it isn't still initialized, in case it gives another chance to check if the active thread is itself.

So let's see some examples of use:

Case #1: Check that one thread ever uses some critical section (recursion allowed)


struct Shared {
void foo() {
WATCH(CriticaSectionA);
bar();
}

void bar() {
WATCH(CriticaSectionA);
}

THREAD_WATCH(CriticaSectionA);
};


Case #2: Check that a class is constructed and destroyed inside the same thread


struct Shared {

Shared() {
WATCH(CTOR_DTOR_SECTION);
...
}

~Shared() {
WATCH(CTOR_DTOR_SECTION);
...
}

THREAD_WATCH(CTOR_DTOR_SECTION);
};

note that doing so the Shared destructor can throw an exception, so do not use this in a production code (put the WATCH between a try-catch and just notify it in some way).

Case #3: Two or more different threads can enter a critical section but in exclusive way (useful to check if external sync mechanism are working).


struct Shared {

foo() {
SCOPED_WATCH(CriticalSectionA);
}


THREAD_WATCH(CriticalSectionA);
};