Hi Peter,

I’ve browsed the new code in your dev branch and here are some remarks/ideas.

TransferTest.cpp:75/80
I think you can get rid of the local variable packetProcessors since it’s used only as a paramter of the Transfer ctor.

Choker.h:71
The Choker:: qualifier is superfluous there.

class PacketProcessor
...
virtual void incomingPacket (const Packet &, TransferSession *) = 0;
virtual void outgoingPacket (const Packet &, TransferSession *) = 0;
...
class TransferSessionStatus
...
virtual void transferSessionReceivedPacket (TransferSession *, const Packet &) = 0;
virtual void transferSessionSendingPacket (TransferSession *, const Packet &) = 0;
...

I was thinking, shouldn’t we unify the TransferSession parameter position? But maybe there is a reason for that it’s not unified…

TransferSessionImpl.cpp:245
I’d replace the implementation with the following one:

Q_ASSERT (list.size() != 0); // To ensure we won't end on a floating point exception in the division below
qint64 sum = std::accomulate (list.begin(), list.end(), 0); // Standard algorithms should be more clearer than raw loops
return sum / list.size();

Ok, that would be everything I’ve noticed. So, what should I focus on now? Maybe implementing the TrackerManager, or do a detailed review of the new code/design/todos, or something else - what do you think?

Regards,

Petr