Code review comment for lp:~widelands-dev/widelands/net-relay

Revision history for this message
SirVer (sirver) wrote :

I added a few more NOCOMs again, sorry :(.

3) That design looks sufficient for what I had in mind. It is also nicely minimal. Good job!

4) Here is my suggestion:

- Change bool try_receive(RecvPacket*) to std::unique_ptr<RecvPacket> try_receive(), with nullptr being what false was before.
- The various std::deque<RecvPacket> received_ can now become std::deque<unique_ptr<RecvPacket>> - which makes them even cheaper.
- You can delete the move operator/constructor in RecvPacket

This has also the advantage that you communicate clearly that a RecvPacket never changes. The move operator makes it look like you sometimes move partially read packets - but you never do as far as I can tell.

« Back to merge proposal