Hot answers tagged

10

Design The problem with your implementation of pop() is that it can not be implemented in an exception safe manner (for any type of T). This is why the standard implementation implements this as two different methods top() and pop(). The top() function simply returns the value while pop() does not return the value but simply removes the value from head. So ...


5

This is sort of an extended comment on Martin York's reply. When you're doing any sort of parallel processing, I advise against re-designing pop so it requires two operations to actually remove an item from the queue, like: T val = queue.top(); queue.pop(); With sufficient care, this can work for a single-producer/single-consumer situation, but has the ...


5

In addition to what has already been said I would add that what happens if s is zero? You might want to throw an exception in the constructor if that happens probably because any call to count of room would fail otherwise. you can definitely improve const correctness at least in a couple of places: in explicit PCQueue(size_t s) : you can make s const. ...


4

Serious issues with this: Memory Management. Not using RAII to lock/unlock Memory Management Don't pass pointers it does not indicate owners. Always wrap pointers in a smart pointer. Have a look at std::unique_ptr. But for types like Data there is no need to use pointers. Simply use Data as the object type (not Data*). RAII Look up the concept of RAII. ...


1

keepAlive.reset is not thread safe. So your class as a whole cannot be thread safe.


1

Looks reasonable to me. But I strongly recommend splitting up some of the more complicated source lines to conform to the rule "one side effect, one line of code." For example: if (--pblock->cnt == 0) delete pblock; This really means uint64_t new_cnt = --pblock->cnt; if (new_cnt == 0) { delete pblock; } Or again: void reset(T* p = nullptr) { ...


Only top voted, non community-wiki answers of a minimum length are eligible